Skip to content

Commit 71babd4

Browse files
ntrel0xEAB
authored andcommitted
[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.
1 parent 03aeafd commit 71babd4

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

std/process.d

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ $(LREF environment), $(LREF thisProcessID) and $(LREF thisThreadID).
9090
module std.process;
9191

9292
import core.thread : ThreadID;
93+
import core.sync.rwmutex;
9394

9495
version (Posix)
9596
{
@@ -160,6 +161,13 @@ private
160161
// Environment variable manipulation.
161162
// =============================================================================
162163

164+
shared ReadWriteMutex mutex;
165+
166+
shared static this()
167+
{
168+
mutex = new shared ReadWriteMutex(mutex.Policy.PREFER_READERS);
169+
}
170+
163171
/**
164172
Manipulates _environment variables using an associative-array-like
165173
interface.
@@ -259,12 +267,14 @@ static:
259267
import std.exception : enforce, errnoEnforce;
260268
if (value is null)
261269
{
270+
// Note: remove needs write lock
262271
remove(name);
263272
return value;
264273
}
265-
if (core.sys.posix.stdlib.setenv(name.tempCString(), value.tempCString(), 1) != -1)
274+
synchronized (mutex.writer)
266275
{
267-
return value;
276+
if (core.sys.posix.stdlib.setenv(name.tempCString(), value.tempCString(), 1) != -1)
277+
return value;
268278
}
269279
// The default errno error message is very uninformative
270280
// in the most common case, so we handle it manually.
@@ -277,6 +287,8 @@ static:
277287
else version (Windows)
278288
{
279289
import std.windows.syserror : wenforce;
290+
291+
synchronized (mutex.writer)
280292
wenforce(
281293
SetEnvironmentVariableW(name.tempCStringW(), value.tempCStringW()),
282294
);
@@ -296,8 +308,9 @@ static:
296308
multi-threaded programs. See e.g.
297309
$(LINK2 https://www.gnu.org/software/libc/manual/html_node/Environment-Access.html#Environment-Access, glibc).
298310
*/
299-
void remove(scope const(char)[] name) @trusted nothrow @nogc
311+
void remove(scope const(char)[] name) @trusted //nothrow @nogc
300312
{
313+
synchronized (mutex.writer)
301314
version (Windows) SetEnvironmentVariableW(name.tempCStringW(), null);
302315
else version (Posix) core.sys.posix.stdlib.unsetenv(name.tempCString());
303316
else static assert(0);
@@ -329,6 +342,8 @@ static:
329342
{
330343
if (name is null)
331344
return false;
345+
346+
synchronized (mutex.reader)
332347
version (Posix)
333348
return core.sys.posix.stdlib.getenv(name.tempCString()) !is null;
334349
else version (Windows)
@@ -363,6 +378,8 @@ static:
363378
{
364379
import std.conv : to;
365380
string[string] aa;
381+
382+
synchronized (mutex.reader)
366383
version (Posix)
367384
{
368385
auto environ = getEnvironPtr;
@@ -428,12 +445,13 @@ private:
428445
// Retrieves the environment variable. Calls `sink` with a
429446
// temporary buffer of OS characters, or `null` if the variable
430447
// doesn't exist.
431-
void getImpl(scope const(char)[] name, scope void delegate(const(OSChar)[]) @safe sink) @trusted
448+
void getImpl(scope const(char)[] name, scope void delegate(scope const(OSChar)[]) @safe sink) @trusted
432449
{
433450
// fix issue https://issues.dlang.org/show_bug.cgi?id=24549
434451
if (name is null)
435452
return sink(null);
436453

454+
synchronized (mutex.reader)
437455
version (Windows)
438456
{
439457
// first we ask windows how long the environment variable is,
@@ -1205,7 +1223,7 @@ private Pid spawnProcessPosix(scope const(char[])[] args,
12051223
}
12061224

12071225
// Execute program.
1208-
core.sys.posix.unistd.execve(argz[0], argz.ptr, envz is null ? getEnvironPtr : envz);
1226+
core.sys.posix.unistd.execve(argz[0], argz.ptr, envz);
12091227

12101228
// If execution fails, exit as quickly as possible.
12111229
abortOnError(forkPipeOut, InternalError.exec, .errno);
@@ -1511,7 +1529,7 @@ private Pid spawnProcessWin(scope const(char)[] commandLine,
15111529
// on the form "name=value", optionally adding those of the current process'
15121530
// environment strings that are not present in childEnv. If the parent's
15131531
// environment should be inherited without modification, this function
1514-
// returns null.
1532+
// returns environ directly.
15151533
version (Posix)
15161534
private const(char*)* createEnv(const string[string] childEnv,
15171535
bool mergeWithParentEnv)
@@ -1521,7 +1539,7 @@ private const(char*)* createEnv(const string[string] childEnv,
15211539
auto environ = getEnvironPtr;
15221540
if (mergeWithParentEnv)
15231541
{
1524-
if (childEnv.length == 0) return null;
1542+
if (childEnv.length == 0) return environ;
15251543
while (environ[parentEnvLength] != null) ++parentEnvLength;
15261544
}
15271545

@@ -1551,7 +1569,16 @@ version (Posix) @system unittest
15511569
assert(e1 != null && *e1 == null);
15521570

15531571
auto e2 = createEnv(null, true);
1554-
assert(e2 == null);
1572+
assert(e2 != null);
1573+
int i = 0;
1574+
auto environ = getEnvironPtr;
1575+
for (; environ[i] != null; ++i)
1576+
{
1577+
assert(e2[i] != null);
1578+
import core.stdc.string : strcmp;
1579+
assert(strcmp(e2[i], environ[i]) == 0);
1580+
}
1581+
assert(e2[i] == null);
15551582

15561583
auto e3 = createEnv(["foo" : "bar", "hello" : "world"], false);
15571584
assert(e3 != null && e3[0] != null && e3[1] != null && e3[2] == null);

0 commit comments

Comments
 (0)