Skip to content

[std.process] Make environment use a ReadWriteMutex #10611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

[std.process] Make environment use a ReadWriteMutex #10611

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
811dacc
[std.process] Make environment use ReadWriteMutex
ntrel Jan 6, 2025
85335a5
Remove commented out attributes for `remove`
ntrel Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions std/process.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ $(LREF environment), $(LREF thisProcessID) and $(LREF thisThreadID).
module std.process;

import core.thread : ThreadID;
import core.sync.rwmutex;

version (Posix)
{
Expand Down Expand Up @@ -160,6 +161,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.
Expand Down Expand Up @@ -259,12 +267,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.
Expand All @@ -277,6 +287,8 @@ static:
else version (Windows)
{
import std.windows.syserror : wenforce;

synchronized (mutex.writer)
wenforce(
SetEnvironmentVariableW(name.tempCStringW(), value.tempCStringW()),
);
Expand All @@ -296,8 +308,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can remove those attributes, this might break code.

Why does this now require GC and throwing? Can we catch any exceptions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the comment for this function should be altered, as now we are supporting multithreaded changes via our own lock.

{
synchronized (mutex.writer)
version (Windows) SetEnvironmentVariableW(name.tempCStringW(), null);
else version (Posix) core.sys.posix.stdlib.unsetenv(name.tempCString());
else static assert(0);
Expand Down Expand Up @@ -329,6 +342,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)
Expand Down Expand Up @@ -363,6 +378,8 @@ static:
{
import std.conv : to;
string[string] aa;

synchronized (mutex.reader)
version (Posix)
{
auto environ = getEnvironPtr;
Expand Down Expand Up @@ -428,12 +445,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,
Expand Down
Loading