5 ms·

The real question is: is there any case where a program calls `setenv` in one thread and actually wants it to take effect in other already-existing threads? Th

by o11c 2y ago

The real question is: is there any case where a program calls `setenv` in one thread and actually wants it to take effect in other already-existing threads? That said, GLIBC is pretty good at documenting all the dangerous functions, so it is possible to add locking/copying yourself.

fweimer 2y ago
Interesting idea. I strongly suspect that there are programs out there that expect that setenv changes the environ array (and they do not treat it as an opaque pointer passed to posix_spawn/execve). With a per-thread setenv, we would need a per-thread environ variable as well. Unfortunately, that's not really compatible with POSIX because environ is not declared in a header. Instead, programmers are expected to write a declaration extern char **environ; into their sources, and that declaration is incompatible with environ being a thread-local variable.
- o11c 2y ago
  Hm, in the end most of the problems do come down to stuff not coming from blessed headers. Regardless of anything else, how about: * deprecate direct access to `environ` and add functions to replace it. Have a macro that indicates this and provide a canonical compatibility shim for people to copy if they might use old libcs. * using linker magic, change the behavior of programs depending on whether they attempt to access `environ` or not, so old-API programs are still thread-unsafe but new ones are thread-safe. It's amazing how much you can do with the conditionally-linked object files from a static "library". Much of C's cross-TU "UB, no diagnostic required" is inexcusable since we can detect it quite easily with zero overhead (at least, for static linking) using today's linkers by deliberately causing multiple definition errors. Compatibility with old-ABI programs probably means fixing `environ` is not that simple, but you are libc and libc is in control of dynamic linking ...
  - ptsneves 2y ago
    All the inconsistencies you suggest sound like a trap, especially when you suggest ABI and API behaviour divergences. From what I understood of your macro idea this would lead to API changes that would lead to ifdefs for different (g)libcurl versions. Doesn’t feel good especially for software distributors. Also the changes you mention require changes in the linker scripts distributed by a variety of toolchains and would need to check if the libc target was a blessed one. In effect the deprecation would never move to obsolete and kept around forever. To clarify the above post libc is in control of dynamic linking through the dl*(dlopen) family of functions.
  - fweimer 2y ago
    Many years ago, glibc did something along those lines for the _res variable (with preprocessor magic instead of linker magic). For the main thread, legacy _res (the actual global data symbol) and new thread-local _res (actually *__res_state()) are the same object, but they diverge for subsequently created threads. I don't think this would work here because it likely changes semantics too much, and not all binaries that need a thread-safe getenv/setenv combination can be rebuilt, especially since compatibility with both variants from the binaries would likely some changes to each application/library.
Asooka 2y ago
Yes, I set up environment variables in a plugin that are later read by already started worker threads. It's not a problem for me because the worker threads are all sleeping on a runqueue, but technically I do want to set an env var in one thread and read it in another that is already running.
deleted 2y ago
[deleted]
kelnos 2y ago
> so it is possible to add locking/copying yourself. Not if third-party dependent libraries use getenv/setenv. (The article mentions this as a continuing problem with the steam client.)
- o11c 2y ago
  As a rule you should not assume third-party libraries are at all thread-safe.
cryptonector 2y ago
Glibc could crib from Illumos, which has a thread-safe putenv()/setenv()/unsetenv()/getenv().
jandrese 2y ago
There are so many better ways to do IPC that this hacky and dangerous getenv/setenv setup is never necessary. I mean what kind of threading library doesn't have shared memory or message passing? I'm guessing this mostly happens in situations where the main process can change variables like HTTPS_PROXY and a different thread is running a library that checks those variables before firing up a TCP socket.

by o11c 2y ago

fweimer 2y ago

Interesting idea. I strongly suspect that there are programs out there that expect that setenv changes the environ array (and they do not treat it as an opaque pointer passed to posix_spawn/execve). With a per-thread setenv, we would need a per-thread environ variable as well. Unfortunately, that's not really compatible with POSIX because environ is not declared in a header. Instead, programmers are expected to write a declaration extern char **environ; into their sources, and that declaration is incompatible with environ being a thread-local variable.

o11c 2y ago
Hm, in the end most of the problems do come down to stuff not coming from blessed headers. Regardless of anything else, how about: * deprecate direct access to `environ` and add functions to replace it. Have a macro that indicates this and provide a canonical compatibility shim for people to copy if they might use old libcs. * using linker magic, change the behavior of programs depending on whether they attempt to access `environ` or not, so old-API programs are still thread-unsafe but new ones are thread-safe. It's amazing how much you can do with the conditionally-linked object files from a static "library". Much of C's cross-TU "UB, no diagnostic required" is inexcusable since we can detect it quite easily with zero overhead (at least, for static linking) using today's linkers by deliberately causing multiple definition errors. Compatibility with old-ABI programs probably means fixing `environ` is not that simple, but you are libc and libc is in control of dynamic linking ...
- ptsneves 2y ago
  All the inconsistencies you suggest sound like a trap, especially when you suggest ABI and API behaviour divergences. From what I understood of your macro idea this would lead to API changes that would lead to ifdefs for different (g)libcurl versions. Doesn’t feel good especially for software distributors. Also the changes you mention require changes in the linker scripts distributed by a variety of toolchains and would need to check if the libc target was a blessed one. In effect the deprecation would never move to obsolete and kept around forever. To clarify the above post libc is in control of dynamic linking through the dl*(dlopen) family of functions.
- fweimer 2y ago
  Many years ago, glibc did something along those lines for the _res variable (with preprocessor magic instead of linker magic). For the main thread, legacy _res (the actual global data symbol) and new thread-local _res (actually *__res_state()) are the same object, but they diverge for subsequently created threads. I don't think this would work here because it likely changes semantics too much, and not all binaries that need a thread-safe getenv/setenv combination can be rebuilt, especially since compatibility with both variants from the binaries would likely some changes to each application/library.

Asooka 2y ago

Yes, I set up environment variables in a plugin that are later read by already started worker threads. It's not a problem for me because the worker threads are all sleeping on a runqueue, but technically I do want to set an env var in one thread and read it in another that is already running.

deleted 2y ago

[deleted]

kelnos 2y ago

> so it is possible to add locking/copying yourself. Not if third-party dependent libraries use getenv/setenv. (The article mentions this as a continuing problem with the steam client.)

o11c 2y ago
As a rule you should not assume third-party libraries are at all thread-safe.

cryptonector 2y ago

Glibc could crib from Illumos, which has a thread-safe putenv()/setenv()/unsetenv()/getenv().

jandrese 2y ago

There are so many better ways to do IPC that this hacky and dangerous getenv/setenv setup is never necessary. I mean what kind of threading library doesn't have shared memory or message passing? I'm guessing this mostly happens in situations where the main process can change variables like HTTPS_PROXY and a different thread is running a library that checks those variables before firing up a TCP socket.