[std.process] Make environment use a ReadWriteMutex
#10611
Conversation
|
Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
|
ntrel
changed the title
environment use a ReadWriteMutex
|
Improper rebase, my bad, will redo. |
Fixes dlang#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.
|
Properly rebased now. |
|
I think it would be better if the final version had braces and indentation around the |
|
For changelog reasons, the commit message would ideally be prefixed with |
| @@ -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 | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Also, the comment for this function should be altered, as now we are supporting multithreaded changes via our own lock.
|
This at least needs a test. |
|
Thinking about this, I'm not sure I like this approach. It depends on implementation details of C, and avoiding calling C functions that might alter the environment. Consider this, if you alter the environment in another thread not through this interface, you get the same problem. Yet the interface is explicitly designed to be able to see such changes. My approach that I recommend is to store a shadow copy of the environment, which updates the environment on Unfortunately, I think this is the only way to keep the functions The nogc thing is a harder problem, but still possible to deal with by using C malloc instead of an AA for storage. What do you think? |
Fixes #10580.
Make getEnvironPtr
@system.Use a ReadWriteMutex to protect reading and writing to
environment.Add
scopetogetImplcallback parameter.Warning 1: This (currently) removes
nothrow @nogcfromremove.Warning 2: I am not that experienced with locks (particularly on Windows), so bear that in mind, I may have done something wrong. Or there may be a better solution, please let me know.