7 ms·

I think I must be missing something here, but I’ll ask anyway: Why don’t the OS libraries have some sort of lock around setenv/getenv, so that only one thread

by jrmg 3y ago

I think I must be missing something here, but I’ll ask anyway: Why don’t the OS libraries have some sort of lock around setenv/getenv, so that only one thread can be inside them at a time? I can’t see how it could deadlock. And surely no-one is so dependent on the performance of these calls that the time to lock/unlock would be problematic?

asveikau 3y ago
I agree. I'm shocked this isn't there in 2023. Or even better, a rwlock allowing concurrent reads while serializing writes. Or some lock free algorithm for writes. I'm pretty sure the Windows version of the environment calls has locking. Historically you can access the environment via a global variable too, which would side-step locking schemes. But probably hardly anybody does that anymore.
- jeroenhd 3y ago
  SetEnvironmentVariable is thread safe on Windows, but their POSIX wrappers aren't. Windows has a better API design in this instance, and the guarantees by POSIX make it impossible to make a compliant implementation that can be used with threads.
  - asveikau 3y ago
    > and the guarantees by POSIX make it impossible to make a compliant implementation that can be used with threads. Can you be more specific than this? I kind of doubt it. For example, seems to me you could write a setenv() that uses a lock-free algorithm or writes in a strategic way that won't result in a fault if getenv() or reading environ(7) runs concurrently, then say all bets are off for thread safety if you write via environ(7). That's safer than the status quo and I don't foresee it breaking POSIX.
    - jeroenhd 3y ago
      Reading the spec again, I suppose it's possible to keep copies of environment variables around in memory without violating the spec, basically creating a copy for every getenv call that was modified since the last setenv/putenv. I thought the spec also specified that writes to the returned pointer would update the environment variables, but no such guarantee is given, that's just an implementation detail (and is disallowed by the API spec but good luck enforcing that). The XSI spec does state that the result of getenv() may be overwritten by putenv() but that's not a strict requirement either. You still risk programs and libraries expecting getenv to always return a pointer to *environ failing (I believe Go has an issue like that on MUSL). On the other hand, the POSIX standard explicitly states that getenv() does not need to be reentrant (and therefore doesn't need to be thread safe) so any program relying on thread-safe getenv is already violating the API contract. The rationale also seems to assume you can't make this stuff thread safe because of the standard implementation: > The getenv() function is inherently not reentrant because it returns a value pointing to static data.
      - asveikau 3y ago
        Modifying the buffer returned by getenv() seems like a terrible way to write back a value, because you could only replace it with a string with equal or shorter length. One of the problems setenv() solves is allocation. It's important to note the difference between reentrant and thread safe. The most obvious implementation of getenv(), which would just loop through environ(7) and do a bunch of strncmp, can safely be re-entered, in that you could interrupt it and call it again and it would produce no ill effect. It just can't be overlapped with writes.
woodruffw 3y ago
I think the missing piece here is how POSIX specifiers the environment: `getenv(3)` and `setenv(3)` are accessors for `environ`[1], which is just a pointer to some memory (over which walking is also specified[2]). That level of granularity means that it's pretty hard to add any locking here. [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1... [2]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/setenv.html https://pubs.opengroup.org/onlinepubs/9699919799/functions/s...
- Retr0id 3y ago
  I wonder if you could get around this by giving each thread its own environment context, and synchronizing them, asynchronously.
  - deleted 3y ago
    [deleted]
  - gpderetta 3y ago
    Just independent environments would be the best, like it was done for locale with uselocale. But it is a breaking change, would have to go through posix and will take forever anyway. Also, as environ is a public symbol, it has ABI implications.
  - tinus_hn 3y ago
    Because there is no way to tell when a thread is done with the buffer, there is no moment when you can be sure you can manipulate it. The options are create a new copy and leak the old one or accept the chance of a segfault.
- jrmg 3y ago
  That makes a lot of sense. You’d need to snapshot environ when the lock was taken (when another thread could be accessing it!), which I imagine would be complicated. Although surely possible.
- YetAnotherNick 3y ago
FridgeSeal 3y ago
The sarcastic answer to that would be something along the lines of “users should be aware of what they’re doing, and you should be more careful about calling those concurrently anyway/good-luck-have-fun”.
- jrmg 3y ago
  I’ve been searching around, and you can find a bunch of discussions about this online. Your ’sarcastic’ argument is basically the one I’ve seen in most places. They could easily be made thread safe, but, paraphrasing, most arguments seem to come down to something like: “setenv and getenv are POSIX functions, and not defined to lock. Just like many POSIX functions, they’ve _never_ been thread safe, and it’s an error to assume they are. Should we really start papering over client errors in use of a supposedly portable API, even though it’s working as specified? And if we make that choice pragmatically for this instance, should we be trying to do it for _all_ of POSIX? That’s impossible for some things, and would add complexity even where it’s not. For all these reasons, it’s better if these just stay dangerous like they’ve always been.”
  - jiggawatts 3y ago
    This kind of historical baggage is one of the main reasons I now completely avoid C/C++ programming and won't touch it ever again. It's Rust or C# only for me from here on...
    - thayne 3y ago
      The problem is that this effects higher languages too, because they often build on libc. And on some OSes, they don't have a choice, because the system call interface is unstable and/or undocumented). For example in rust, multiple time libraries were found to be unsound if `std::env::set_env` was ever called from a multi-threaded program. See: https://github.com/time-rs/time/issues/293 https://github.com/time-rs/time/issues/293 and https://github.com/chronotope/chrono/issues/499 https://github.com/chronotope/chrono/issues/499 https://github.com/rust-lang/rust/issues/27970 https://github.com/rust-lang/rust/issues/27970 https://github.com/rust-lang/rust/issues/90308 https://github.com/rust-lang/rust/issues/90308
raggi 3y ago
there are lots of programs that access `*environ` directly, so while this might be good, it wouldn’t solve all classes of the problem. There are also uses out there which are performance sensitive (and often just as if not more unsafe, such as holding pointers into the structure over long periods). threaded programs should probably seriously consider retiring libc, but we don’t currently have a common ground replacement. name related activities are one of the worst areas, contributing significantly to the glibc linkage and abi challenges, but also lacking sufficient standards for alternatives or even consensus to be built quickly.
- dundarious 3y ago
  Yes, on Linux you can even write `int main(int argc, char* argv[], char* envp[])`.
  - o11c 3y ago
    `envp` is unsafe after `setenv` even in single-threaded programs though. So you really should use `environ`.
dataflow 3y ago
That's why getenv_s et al were added.
JoshTriplett 3y ago
There's a proposal for that: https://www.owlfolio.org/development/thread-safe-environment-variable-mutation-working-draft-2022-15/ https://www.owlfolio.org/development/thread-safe-environment...
- thayne 3y ago
  That would be nice. What is the likelihood this will actually make it into libc implementations and/or become part of the posix standard?
wahern 3y ago
getenv returns a pointer which could be invalidated after releasing the lock, unless the lock also guards uses of that pointer and all application code uses that lock, which they most certainly do not. Likewise, this scheme does not solve direct use of environ by application code. NetBSD has getenv_r, which copies into a buffer, but few applications use getenv_r, and certainly not all of them. And it doesn't resolve environ. Solaris never free's env strings or environ arrays, only creating new copies and atomically swapping them. It uses a special allocator for those objects which doubles the backing buffer each time it deep copies the environ array, then argues this strategy is technically asymptotically memory bounded. EDIT: Glancing at the code I think glibc is similar to Solaris in that it never free's env strings, but it has a heuristic to conditionally free environ arrays which means directly using environ isn't thread-safe.
- thayne 3y ago
  Having a lock would still be better than the current situation. Especially if the lock was exposed so that programs that did mess with environ directly, or the pointer returned by getenv could hold the lock while doing so.
  - lazide 3y ago
    Deadlock disaster that one.
- cryptonector 3y ago
  `getenv()` never returns pointers that can be invalidated. If you `putenv()` something, you commit to never freeing it or overwriting it.
  - hddqsb 3y ago
    Good point about `putenv()`; however there is also `setenv()`, which does make a copy, so you are wrong about `getenv()` in general. POSIX explicitly states "The string [returned by getenv] may be overwritten by a subsequent call to getenv(), setenv(), unsetenv(), or putenv()" (https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/getenv.html https://pubs.opengroup.org/onlinepubs/9699919799.2008edition...).
forrestthewoods 3y ago
If I were King I would ban environment variables from the OS entirely. Global mutable state is the root of all evil! Globals are evil evil evil and the modern reliance on bullshit environment variables is a plague upon reliability. Kill it all with fire.
- gpderetta 3y ago
  Should we also get rid of filesystems? Databases? All form of RPC?
  - nikanj 3y ago
    Honestly sometimes I think the answer is yes. Imagine how happy we could be, and how many fewer problems we would have. Add printers to that list and you're describing a paradise.
  - forrestthewoods 3y ago
    I definitely think a lot of filesystem access is a code smell and probably not the right thing. That one causes me a lot of pain. But that’s largely because I work in games and you really need to use the Unity/Unreal/whatever asset management system instead of direct file system access. I’ve got a small build system and the first thing it does is nuke PATH to empty. It’s glorious. No more grabbing random shit from a big blob with untracked dependencies that varies wildly by system! I could easily live my entire life without environment variables. They’re just a fundamentally bad idea. Every program that foolishly uses environment variables can be replaced by a better program that takes a config file or arglist.
- bayindirh 3y ago
  Well, environment variables are not "global" globals. They are just my globals, or my post-it notes for some variables. Because they are not per-user even. They are per user session. 10 processes can have completely different set of values for the same environment variables, because they are in their own environments, and apparently, that's useful. There are foot guns, and there are unintentional consequences of implementation and design details. This is why we patch, improve and rewrite our software over time. To iron out these kinks. Fire is also have a tendency to cause collateral damage. So use both fire and environment variables responsibly, and world will be a better place.
jclulow 3y ago
On at least some other operating systems, getenv(3C) and setenv(3C) are indeed thread-safe; e.g., on illumos: https://illumos.org/man/3C/getenv https://illumos.org/man/3C/getenv We inherited our implementation from OpenSolaris, as did Oracle when they created their Solaris 11 fork. I expect at least some of the BSDs have probably fixed this as well.
- o11c 3y ago
  Just because it is documented as thread safe doesn't mean it actually is. They might have just not understood the problem (see e.g. the various indirect links to "please mandate a thread-local `getenv`"). `setenv` is a nasty beast, especially since the raw `environ` variable is also exposed (and is in fact the only way to enumerate the environment).
  - tjfontaine 3y ago
    In my experience, because it is documented that’s the behavior I would expect. That being said, I went to look[0] and it turns out it wasn’t a lie. [0]https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/getenv.c https://github.com/illumos/illumos-gate/blob/master/usr/src/...
    - hddqsb 3y ago
      For the curious: They make getenv() thread-safe by intentionally leaking the old environment, which they argue is acceptable because the memory leak is bounded to 3x the space actually needed. The getenv/setenv/putenv/environ API looks terrible on closer inspection -- it does not appear possible for an implementation to be safe, leak-free, and efficient.
    - o11c 3y ago
      Just because they use locks doesn't mean it's automatically actually thread-safe. The comment about "this is safe for people walking `environ`" is definitely a lie, for example, though the bug might be hidden on some common architectures with popular compilers in their default configuration.
cryptonector 3y ago
On Solaris/Illumos `putenv()` and `getenv()` are lock-less and really fast. Basically, if you `putenv()`, you commit to never freeing that memory.
- xy2_ 3y ago
  putenv is not lock free: https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/getenv.c#L222C2-L222C13 https://github.com/illumos/illumos-gate/blob/master/usr/src/...
  - cryptonector 3y ago
    Ay, yes. Though it could be made lock-less.
gpderetta 3y ago
Even if the api was perfect and used locks and returned memory to be managed by the caller, it would still be hard to use safely in a multithreading environment as long as the env is a process global property.

by jrmg 3y ago

asveikau 3y ago

I agree. I'm shocked this isn't there in 2023. Or even better, a rwlock allowing concurrent reads while serializing writes. Or some lock free algorithm for writes. I'm pretty sure the Windows version of the environment calls has locking. Historically you can access the environment via a global variable too, which would side-step locking schemes. But probably hardly anybody does that anymore.

jeroenhd 3y ago
SetEnvironmentVariable is thread safe on Windows, but their POSIX wrappers aren't. Windows has a better API design in this instance, and the guarantees by POSIX make it impossible to make a compliant implementation that can be used with threads.
- asveikau 3y ago
  > and the guarantees by POSIX make it impossible to make a compliant implementation that can be used with threads. Can you be more specific than this? I kind of doubt it. For example, seems to me you could write a setenv() that uses a lock-free algorithm or writes in a strategic way that won't result in a fault if getenv() or reading environ(7) runs concurrently, then say all bets are off for thread safety if you write via environ(7). That's safer than the status quo and I don't foresee it breaking POSIX.
  - jeroenhd 3y ago
    Reading the spec again, I suppose it's possible to keep copies of environment variables around in memory without violating the spec, basically creating a copy for every getenv call that was modified since the last setenv/putenv. I thought the spec also specified that writes to the returned pointer would update the environment variables, but no such guarantee is given, that's just an implementation detail (and is disallowed by the API spec but good luck enforcing that). The XSI spec does state that the result of getenv() may be overwritten by putenv() but that's not a strict requirement either. You still risk programs and libraries expecting getenv to always return a pointer to *environ failing (I believe Go has an issue like that on MUSL). On the other hand, the POSIX standard explicitly states that getenv() does not need to be reentrant (and therefore doesn't need to be thread safe) so any program relying on thread-safe getenv is already violating the API contract. The rationale also seems to assume you can't make this stuff thread safe because of the standard implementation: > The getenv() function is inherently not reentrant because it returns a value pointing to static data.
    - asveikau 3y ago
      Modifying the buffer returned by getenv() seems like a terrible way to write back a value, because you could only replace it with a string with equal or shorter length. One of the problems setenv() solves is allocation. It's important to note the difference between reentrant and thread safe. The most obvious implementation of getenv(), which would just loop through environ(7) and do a bunch of strncmp, can safely be re-entered, in that you could interrupt it and call it again and it would produce no ill effect. It just can't be overlapped with writes.

woodruffw 3y ago

I think the missing piece here is how POSIX specifiers the environment: `getenv(3)` and `setenv(3)` are accessors for `environ`[1], which is just a pointer to some memory (over which walking is also specified[2]). That level of granularity means that it's pretty hard to add any locking here. [1]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1... [2]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/setenv.html https://pubs.opengroup.org/onlinepubs/9699919799/functions/s...

Retr0id 3y ago
I wonder if you could get around this by giving each thread its own environment context, and synchronizing them, asynchronously.
- deleted 3y ago
  [deleted]
- gpderetta 3y ago
  Just independent environments would be the best, like it was done for locale with uselocale. But it is a breaking change, would have to go through posix and will take forever anyway. Also, as environ is a public symbol, it has ABI implications.
- tinus_hn 3y ago
  Because there is no way to tell when a thread is done with the buffer, there is no moment when you can be sure you can manipulate it. The options are create a new copy and leak the old one or accept the chance of a segfault.
jrmg 3y ago
That makes a lot of sense. You’d need to snapshot environ when the lock was taken (when another thread could be accessing it!), which I imagine would be complicated. Although surely possible.
YetAnotherNick 3y ago

FridgeSeal 3y ago

The sarcastic answer to that would be something along the lines of “users should be aware of what they’re doing, and you should be more careful about calling those concurrently anyway/good-luck-have-fun”.

jrmg 3y ago
I’ve been searching around, and you can find a bunch of discussions about this online. Your ’sarcastic’ argument is basically the one I’ve seen in most places. They could easily be made thread safe, but, paraphrasing, most arguments seem to come down to something like: “setenv and getenv are POSIX functions, and not defined to lock. Just like many POSIX functions, they’ve _never_ been thread safe, and it’s an error to assume they are. Should we really start papering over client errors in use of a supposedly portable API, even though it’s working as specified? And if we make that choice pragmatically for this instance, should we be trying to do it for _all_ of POSIX? That’s impossible for some things, and would add complexity even where it’s not. For all these reasons, it’s better if these just stay dangerous like they’ve always been.”
- jiggawatts 3y ago
  This kind of historical baggage is one of the main reasons I now completely avoid C/C++ programming and won't touch it ever again. It's Rust or C# only for me from here on...
  - thayne 3y ago
    The problem is that this effects higher languages too, because they often build on libc. And on some OSes, they don't have a choice, because the system call interface is unstable and/or undocumented). For example in rust, multiple time libraries were found to be unsound if `std::env::set_env` was ever called from a multi-threaded program. See: https://github.com/time-rs/time/issues/293 https://github.com/time-rs/time/issues/293 and https://github.com/chronotope/chrono/issues/499 https://github.com/chronotope/chrono/issues/499 https://github.com/rust-lang/rust/issues/27970 https://github.com/rust-lang/rust/issues/27970 https://github.com/rust-lang/rust/issues/90308 https://github.com/rust-lang/rust/issues/90308

raggi 3y ago

there are lots of programs that access `*environ` directly, so while this might be good, it wouldn’t solve all classes of the problem. There are also uses out there which are performance sensitive (and often just as if not more unsafe, such as holding pointers into the structure over long periods). threaded programs should probably seriously consider retiring libc, but we don’t currently have a common ground replacement. name related activities are one of the worst areas, contributing significantly to the glibc linkage and abi challenges, but also lacking sufficient standards for alternatives or even consensus to be built quickly.

dundarious 3y ago
Yes, on Linux you can even write `int main(int argc, char* argv[], char* envp[])`.
- o11c 3y ago
  `envp` is unsafe after `setenv` even in single-threaded programs though. So you really should use `environ`.

dataflow 3y ago

That's why getenv_s et al were added.

JoshTriplett 3y ago

There's a proposal for that: https://www.owlfolio.org/development/thread-safe-environment-variable-mutation-working-draft-2022-15/ https://www.owlfolio.org/development/thread-safe-environment...

thayne 3y ago
That would be nice. What is the likelihood this will actually make it into libc implementations and/or become part of the posix standard?

wahern 3y ago

getenv returns a pointer which could be invalidated after releasing the lock, unless the lock also guards uses of that pointer and all application code uses that lock, which they most certainly do not. Likewise, this scheme does not solve direct use of environ by application code. NetBSD has getenv_r, which copies into a buffer, but few applications use getenv_r, and certainly not all of them. And it doesn't resolve environ. Solaris never free's env strings or environ arrays, only creating new copies and atomically swapping them. It uses a special allocator for those objects which doubles the backing buffer each time it deep copies the environ array, then argues this strategy is technically asymptotically memory bounded. EDIT: Glancing at the code I think glibc is similar to Solaris in that it never free's env strings, but it has a heuristic to conditionally free environ arrays which means directly using environ isn't thread-safe.

thayne 3y ago
Having a lock would still be better than the current situation. Especially if the lock was exposed so that programs that did mess with environ directly, or the pointer returned by getenv could hold the lock while doing so.
- lazide 3y ago
  Deadlock disaster that one.
cryptonector 3y ago
`getenv()` never returns pointers that can be invalidated. If you `putenv()` something, you commit to never freeing it or overwriting it.
- hddqsb 3y ago
  Good point about `putenv()`; however there is also `setenv()`, which does make a copy, so you are wrong about `getenv()` in general. POSIX explicitly states "The string [returned by getenv] may be overwritten by a subsequent call to getenv(), setenv(), unsetenv(), or putenv()" (https://pubs.opengroup.org/onlinepubs/9699919799.2008edition/functions/getenv.html https://pubs.opengroup.org/onlinepubs/9699919799.2008edition...).

forrestthewoods 3y ago

If I were King I would ban environment variables from the OS entirely. Global mutable state is the root of all evil! Globals are evil evil evil and the modern reliance on bullshit environment variables is a plague upon reliability. Kill it all with fire.

gpderetta 3y ago
Should we also get rid of filesystems? Databases? All form of RPC?
- nikanj 3y ago
  Honestly sometimes I think the answer is yes. Imagine how happy we could be, and how many fewer problems we would have. Add printers to that list and you're describing a paradise.
- forrestthewoods 3y ago
  I definitely think a lot of filesystem access is a code smell and probably not the right thing. That one causes me a lot of pain. But that’s largely because I work in games and you really need to use the Unity/Unreal/whatever asset management system instead of direct file system access. I’ve got a small build system and the first thing it does is nuke PATH to empty. It’s glorious. No more grabbing random shit from a big blob with untracked dependencies that varies wildly by system! I could easily live my entire life without environment variables. They’re just a fundamentally bad idea. Every program that foolishly uses environment variables can be replaced by a better program that takes a config file or arglist.
bayindirh 3y ago
Well, environment variables are not "global" globals. They are just my globals, or my post-it notes for some variables. Because they are not per-user even. They are per user session. 10 processes can have completely different set of values for the same environment variables, because they are in their own environments, and apparently, that's useful. There are foot guns, and there are unintentional consequences of implementation and design details. This is why we patch, improve and rewrite our software over time. To iron out these kinks. Fire is also have a tendency to cause collateral damage. So use both fire and environment variables responsibly, and world will be a better place.

jclulow 3y ago

On at least some other operating systems, getenv(3C) and setenv(3C) are indeed thread-safe; e.g., on illumos: https://illumos.org/man/3C/getenv https://illumos.org/man/3C/getenv We inherited our implementation from OpenSolaris, as did Oracle when they created their Solaris 11 fork. I expect at least some of the BSDs have probably fixed this as well.

o11c 3y ago
Just because it is documented as thread safe doesn't mean it actually is. They might have just not understood the problem (see e.g. the various indirect links to "please mandate a thread-local `getenv`"). `setenv` is a nasty beast, especially since the raw `environ` variable is also exposed (and is in fact the only way to enumerate the environment).
- tjfontaine 3y ago
  In my experience, because it is documented that’s the behavior I would expect. That being said, I went to look[0] and it turns out it wasn’t a lie. [0]https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/getenv.c https://github.com/illumos/illumos-gate/blob/master/usr/src/...
  - hddqsb 3y ago
    For the curious: They make getenv() thread-safe by intentionally leaking the old environment, which they argue is acceptable because the memory leak is bounded to 3x the space actually needed. The getenv/setenv/putenv/environ API looks terrible on closer inspection -- it does not appear possible for an implementation to be safe, leak-free, and efficient.
  - o11c 3y ago
    Just because they use locks doesn't mean it's automatically actually thread-safe. The comment about "this is safe for people walking `environ`" is definitely a lie, for example, though the bug might be hidden on some common architectures with popular compilers in their default configuration.

cryptonector 3y ago

On Solaris/Illumos `putenv()` and `getenv()` are lock-less and really fast. Basically, if you `putenv()`, you commit to never freeing that memory.

xy2_ 3y ago
putenv is not lock free: https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/port/gen/getenv.c#L222C2-L222C13 https://github.com/illumos/illumos-gate/blob/master/usr/src/...
- cryptonector 3y ago
  Ay, yes. Though it could be made lock-less.

gpderetta 3y ago

Even if the api was perfect and used locks and returned memory to be managed by the caller, it would still be hard to use safely in a multithreading environment as long as the env is a process global property.