From ba0de15f7604c2773da6f0ccd7375242ebe5535b Mon Sep 17 00:00:00 2001
From: Nick Treleaven <ntrel002@gmail.com>
Date: Mon, 6 Jan 2025 20:28:55 +0000
Subject: [PATCH] [std.process] Make environment use ReadWriteMutex

Fixes #10580.

Make getEnvironPtr `@system`.
Use a ReadWriteMutex to protect reading and writing to environment.
Add `scope` to `getImpl` callback parameter.

Warning 1: This (currently) removes `nothrow @nogc` from `remove`.
Warning 2: I am not that experienced with locks, so bear that in mind,
I may have done something wrong. Or there may be a better solution,
please let me know.
---
 std/process.d | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/std/process.d b/std/process.d
index 1cb2264f6d3..3d6d553a390 100644
--- a/std/process.d
+++ b/std/process.d
@@ -90,6 +90,7 @@ $(LREF environment), $(LREF thisProcessID) and $(LREF thisThreadID).
 module std.process;
 
 import core.thread : ThreadID;
+import core.sync.rwmutex;
 
 version (Posix)
 {
@@ -149,7 +150,7 @@ private
         version (Darwin)
         {
             extern(C) char*** _NSGetEnviron() nothrow;
-            const(char**) getEnvironPtr() @trusted
+            const(char**) getEnvironPtr() @system
             {
                 return *_NSGetEnviron;
             }
@@ -158,7 +159,7 @@ private
         {
             // Made available by the C runtime:
             extern(C) extern __gshared const char** environ;
-            const(char**) getEnvironPtr() @trusted
+            const(char**) getEnvironPtr() @system
             {
                 return environ;
             }
@@ -176,6 +177,13 @@ private
 // Environment variable manipulation.
 // =============================================================================
 
+shared ReadWriteMutex mutex;
+
+shared static this()
+{
+    mutex = new shared ReadWriteMutex(mutex.Policy.PREFER_READERS);
+}
+
 /**
 Manipulates _environment variables using an associative-array-like
 interface.
@@ -275,12 +283,14 @@ static:
             import std.exception : enforce, errnoEnforce;
             if (value is null)
             {
+                // Note: remove needs write lock
                 remove(name);
                 return value;
             }
-            if (core.sys.posix.stdlib.setenv(name.tempCString(), value.tempCString(), 1) != -1)
+            synchronized (mutex.writer)
             {
-                return value;
+                if (core.sys.posix.stdlib.setenv(name.tempCString(), value.tempCString(), 1) != -1)
+                    return value;
             }
             // The default errno error message is very uninformative
             // in the most common case, so we handle it manually.
@@ -293,6 +303,8 @@ static:
         else version (Windows)
         {
             import std.windows.syserror : wenforce;
+
+            synchronized (mutex.writer)
             wenforce(
                 SetEnvironmentVariableW(name.tempCStringW(), value.tempCStringW()),
             );
@@ -312,8 +324,9 @@ static:
     multi-threaded programs. See e.g.
     $(LINK2 https://www.gnu.org/software/libc/manual/html_node/Environment-Access.html#Environment-Access, glibc).
     */
-    void remove(scope const(char)[] name) @trusted nothrow @nogc
+    void remove(scope const(char)[] name) @trusted //nothrow @nogc
     {
+        synchronized (mutex.writer)
         version (Windows)    SetEnvironmentVariableW(name.tempCStringW(), null);
         else version (Posix) core.sys.posix.stdlib.unsetenv(name.tempCString());
         else static assert(0);
@@ -345,6 +358,8 @@ static:
     {
         if (name is null)
             return false;
+
+        synchronized (mutex.reader)
         version (Posix)
             return core.sys.posix.stdlib.getenv(name.tempCString()) !is null;
         else version (Windows)
@@ -379,6 +394,8 @@ static:
     {
         import std.conv : to;
         string[string] aa;
+
+        synchronized (mutex.reader)
         version (Posix)
         {
             auto environ = getEnvironPtr;
@@ -444,12 +461,13 @@ private:
     // Retrieves the environment variable. Calls `sink` with a
     // temporary buffer of OS characters, or `null` if the variable
     // doesn't exist.
-    void getImpl(scope const(char)[] name, scope void delegate(const(OSChar)[]) @safe sink) @trusted
+    void getImpl(scope const(char)[] name, scope void delegate(scope const(OSChar)[]) @safe sink) @trusted
     {
         // fix issue https://issues.dlang.org/show_bug.cgi?id=24549
         if (name is null)
             return sink(null);
 
+        synchronized (mutex.reader)
         version (Windows)
         {
             // first we ask windows how long the environment variable is,