20 ms·

Setenv Is Not Thread Safe and C Doesn't Want to Fix It

• turtleyacht 3y ago
  Should apps orchestrate a super-global lock of a foreign namespace? An environment variable's value, for a running process, is just what it is: an initial value from outside. Adding complexity around it smells like an attempt to control a distributed mutex, like checking an API for real-time value changes in a while loop across several instances of the same app. I thought there would be alternatives to this, like pubsub, Kafka, or other asynchronous event handling. Imagine having to test an app for its ability to handle safe read-write of OS-level state. It's definitionally bankrupt: not really a unit, not easy to set up quickly, and not isolated.
  • xxs 3y ago
    >Imagine having to test an app for its ability to handle safe read-write of OS-level state. I Also it should be able to handle invariants as modifying multiple variables is not an atomic process, either.
  • aidenn0 3y ago
    > An environment variable's value, for a running process, is just what it is: an initial value from outside. Then calling setenv() from anywhere except in the time between a fork() and exec() call should be banned, but it's not. Honestly, calling abort() if setenv is called in the presence of threads would seem like a better status-quo than what we have today.
    • nwmcsween 3y ago
      And how do you propose banning it? Should libc track function calls? Maybe when the linker links setenv we just print out the man page for setenv.
• Khelavaster 3y ago
  Slap a mutex on that beast! This is expected behavior for setting a GLOBAL variable without a lock on the memoryspace..
  • krackers 3y ago
    You can't guarantee that whatever libraries you pull in use the same mutex as you though.
    • JaDogg 3y ago
      who pull in libraries in C like this?
      • agevag 3y ago
        [dead]
      • Tobu 3y ago
        Who doesn't? libc itself calls getenv when getting system time: https://news.ycombinator.com/item?id=38344224 https://news.ycombinator.com/item?id=38344224 You may have a mutex on getenv/setenv, like the Rust stdlib does, but when libc doesn't look at that mutex, even on the read side, you run into UB. So the next step is never calling into seemingly innocent libc functions in safe code (which you have to enforce on your dependencies as well), implementing safe alternatives to a good chunk of libc (and making sure your dependencies use those), to cordon off anything that looks at the environment. This makes a good chunk of POSIX functionality useless.
      • JaDogg 3y ago
        OK thank you for the explanation.. This makes more more scared to bring in libraries :(
    • seeknotfind 3y ago
      I thought they were talking about inside the libc implementation. Though, if that was done and people call getenv from async contexts, it could deadlock.
• pjmlp 3y ago
  WG14 also doesn't want to provide safer string and array manipulation libraries for decades, even Dennis Ritchie failed to get fat pointers into ISO C, why should fixing this be any different?
  • raverbashing 3y ago
    Yeah, at this point the languages and OSs should (IMHO) be distancing them from the multiple craziness of the committee
• grandinj 3y ago
  The right thing to do is to create a new thread safe api and implement that and then standardize it. Hard but very unsexy work.
  • xxs 3y ago
    The right thing is not to modify a global state once you have started all the threads. If you need to do that, use your own data structures. The case of getaddrinfo - would need a copy of the entire env. in a thread safe manner, then return the result. That would pretty much apply to anything that uses env
  • saagarjha 3y ago
    The person who fixed this would be incredibly sexy in my book. Though, that probably tells you more about my own sexiness than it invalidates your comment.
    • xxs 3y ago
      The sexy part is more like having an extremely brushed/retouched image in a magazine - it's not real.
    • cryptonector 3y ago
      It is fixed in Solaris/Illumos. glibc just needs to copy the approach.
• DangerousDoctor 3y ago
  The essential problem is that there is no thread-safe way to implement this while maintaining backwards compatibility -- applications can alter the environment block by changing the environ global pointer, applications can also alter the environment block by replacing individual pointers in the environ array, applications can also alter the environment block by altering the strings pointed to by the individual members of the environ array, applications can also alter the environment block by using setenv/putenv/etc. Inserting a mutex into the setenv/getenv/etc. functions is pointless because applications are explicitly allowed to modify the environ pointer and array directly without any locking.
  • forrestthewoods 3y ago
    Yup. It's a bad, broken, and unfixable API. The world really needs a new C standard library that doesn't suck.
    • eviks 3y ago
      Or transition to post-C
      • xxs 3y ago
        Java has System.getenv()... and it's an unmodifiable Map<String, String>. The real culprit it's the attempt to modify it, not C.
    • rewmie 3y ago
      > Yup. It's a bad, broken, and unfixable API. Is it, though? The only argument I see is that an API can be misused. There are ergonomics debates we can have about it, but a user intentionally abusing an API wanting to do something that's completely wrong is hardly indication the API is broken.
      • forrestthewoods 3y ago
        Exposing global mutable access to something and providing no thread safe version counts as broken in my book. Your book may differ. A good API would probably only allow constant access. If mutation is for some reason deemed necessary then it should be through a separate set API and the return results of get should be guaranteed safe.
• krackers 3y ago
  Cool, ever since that rachelbythebay article [1] I was wondering how different libcs handle the issue! Nice to see that someone else confirming the behavior of apple'c libc. It's not mentioned in the article, but while apple's libc seems to suffer from the use-after-free issue, if I'm reading it right it does seem to have locking for setenv/getenv [3] [1] https://news.ycombinator.com/item?id=37908655 https://news.ycombinator.com/item?id=37908655 [2] https://news.ycombinator.com/item?id=37952916 https://news.ycombinator.com/item?id=37952916 [3] https://github.com/apple-open-source-mirror/Libc/blob/master/stdlib/FreeBSD/getenv.c https://github.com/apple-open-source-mirror/Libc/blob/master...
• xxs 3y ago
  The described problem (thread safety) for a global configuration seems mostly a misunderstanding by the author. The usual case for modifying a global state is: modify once, then proceed (e.g. start new threads). Even if all the calls become thread safe, the behavior would be inconsistent, still.
  • hddqsb 3y ago
    It is perfectly reasonable and consistent for one thread to set an environment variable while other threads are reading different environment variables.
• rpcope1 3y ago
  I wonder if you could work around this by using LD_PRELOAD to load in a shim around get_env and set_env. You'd still have the problem of environ potentially getting mutated, but it very well may solve the problem if it's limited to those two functions.
  • jeroenhd 3y ago
    You would still need to design a fix. You'll probably either break programs that modify the pointer returned by getenv() while doing so. However, this only makes sense for other people's software crashing on getenv() related memory bugs. If you control the software, you can simply prevent the setenv() call yourself. No need to LD_PRELOAD anything, just load a library or write your own hooking code to work around the POSIX madness.
• eqvinox 3y ago
  > This is a list of some uses of environment variables from fairly widely used libraries and services. This shows that environment variables are pretty widely used. Widely used, yes. Used as in read. Why do any of these need to change at runtime? And if they do - why are they environment variables? (NB: starting a new process is not "at runtime")
  • withinboredom 3y ago
    Changing the env during runtime is actually quite handy for debugging and forcing the program into specific states. Other than that, it can also be handy in k8s with a VPA. You get more/less memory and then update the env to reflect that. Your service picks up the env change and updates the runtime. IIRC, there is/was some way to listen to those changes in C#, and automatically update runtime settings.
    • eqvinox 3y ago
      > Other than that, it can also be handy in k8s with a VPA. You get more/less memory and then update the env to reflect that. Your service picks up the env change and updates the runtime. You… can't change the env from outside the process… are you saying this is used by disjoint components within a single process? Or is this just a misunderstanding?
      • withinboredom 3y ago
        You can spawn as many processes as you want in a container, did you not know that? But you only need access to the /proc/pid directory to change another processes env.
        • eqvinox 3y ago
          > But you only need access to the /proc/pid directory to change another processes env. /proc/$pid/environ is not writable (and as a matter of fact, due to how the environment works, it cannot be writable.)
• pitdicker 3y ago
  This also caused a lot of trouble for time libraries in Rust. The two foundational libraries, chrono and time, rely on localtime_r to get the local time instead of the clock value in UTC. localtime_r reads the TZ environment variable (and optionally others like TZ_DIR). Rust declares it safe to modify the environment, while POSIX declares it unsafe. CVE-2020-26235, RUSTSEC-2020-0071 and RUSTSEC-2020-0159 where opened against the crates. That left the Rust ecosystem with a pretty much unsolvable issue for many months. Chrono went with the solution to parse the timezone database of the OS natively and read the environment using the Rust locks. Time tries to detect if the libc version has thread-safety guarantees to access the environment, and otherwise panics if there are multiple threads. More reading: https://docs.rs/chrono/latest/chrono/#security-advisories https://docs.rs/chrono/latest/chrono/#security-advisories
  • rewmie 3y ago
    > Rust declares it safe to modify the environment, while POSIX declares it unsafe. There's your problem right there, and it ain't the behavior specified in the standard.
    • pitdicker 3y ago
      You are right. POSIX specifies one thing, the standard library in Rust and some other libraries specifies something different. 'Safe to use unless there are other threads' is not really something you can or want to encode in a type system. But libraries and users are caught in the middle.
      • eptcyka 3y ago
        It is safe to use the Rust standard library interface.
        • pitdicker 3y ago
          Unless the environment is also touched by a part of the program written in Go, Julia, I don't know... The lock is not shared across languages.
• vintermann 3y ago
  Using environment variables for global mutable state isn't exactly good practice, is it? I can't think of any time I had wanted to do that. What exactly are the programs that break if this changes?
  • okr 3y ago
    I can think of using libraries, that get their config through environment variables. So you start your program, modify environment, and then start up the rest.
    • quickthrower2 3y ago
      Does the “and then” protect you here?
      • okr 3y ago
        No, not really. You never know when the library reads the enviroment variables. But i can make sure, that the parts under my control do not modify them after start.
    • johanbcn 3y ago
      Libraries shouldn't be reading environment variables, that responsibility should be only for the application.
      • kuon 3y ago
        It is common, out of my head SDL and pipewire do it.
      • lelanthran 3y ago
        They shouldn't, but if even the C standard library reads environment variables, then the bar is set pretty low for library designers. Look up the mess around LOCALE sometime. The best the programmer can do is perform all the setenv calls before spawning any threads or making any library calls.
• Someone 3y ago
  I may misunderstand what “Extension to the ISO C standard” can mean, but getenv isn’t thread-safe, either. https://pubs.opengroup.org/onlinepubs/9699919799/ https://pubs.opengroup.org/onlinepubs/9699919799/: “The getenv() function need not be thread-safe” I expect most if not all implementations are more robust.
  • numeromancer 3y ago
    It says more than that: "The returned string pointer might be invalidated or the string content might be overwritten by a subsequent call to getenv()" You don't even need threads for this to be unsafe; another call to the same function may invalidate earlier-gotten pointers. I don't see how to interpret this as anything but broken.
    • Someone 3y ago
      I think it’s borderline broken. Such APIs are defensible if you don’t have threads and memory is very scarce. Reading https://man.freebsd.org/cgi/man.cgi?getenv(3) https://man.freebsd.org/cgi/man.cgi?getenv(3) it was introduced in Version 7 AT&T UNIX (https://en.wikipedia.org/wiki/Version_7_Unix https://en.wikipedia.org/wiki/Version_7_Unix). That didn’t have threads. Now, was memory scarce? The PDP-11 it was designed for could have megabytes of RAM, but I wouldn’t know what the smallest machines this ran on had, and of course, those systems were multi-user. So, maybe, thisxwas a good choice, but it also could be an early example of using a small system, hacker mindset on machines that didn’t need that anymore.
      • numeromancer 3y ago
        The text implies that any second call to getenv invalidates the returned value of any prior call to getenv; even without threads, without calling setenv, a second call to the function invalidates the return from an earlier call. I re-affirm my conclusion, without qualification: BROKEN.
• yason 3y ago
  Come on! Of all C/Posix thread-safety issues in circulation this can plausibly be considered the most moot one. Environment variables are not meant to be an inter-thread communication channel and the documentation that points out setenv() is not thread-safe is very much a fair shot. You rarely, if ever, need to setenv() anything maybe unless you're a shell. For spawning children execve() already takes an envp parameter. For debugging I think I've mostly set in-process environment variables manually from gdb. Further, because environment variables are an interface between the process and its environment you typically read environment variables at start and cache the parsed values in some internal location. If you need to change that global state on the go you should do it using your own internal variables instead of recycling it through the environment and having the program threads repeatedly getenv() the updated values.
• usrbinbash 3y ago
  > C Doesn't Want to Fix It Or: C knows that it doesn't need fixing. How often do I need to `setenv()` anything? The answer is "Never" in the vast majority of programs, because ENVVRS are usually read rather than set, so this issue is nonexistent for them. For the vast majority of the small amount of programs that actually need to use `setenv()`, the answer is: "Maybe once or twice during the entire lifetime of the process, and then only at the very start, probably even before running any threads", meaning this issue is nonexistent for them as well. So, is there a potential issue with thread safetey? Yes. Does it matter given where and under what circumstances it occurs? Not really. > such as Go's os.Setenv (Go issue) Here is the link to the "issue": https://github.com/golang/go/issues/63567 https://github.com/golang/go/issues/63567 What kind of actual real life production code would continuously set envvars while simutaneously calling a function that tries to read the environment? Yes, this is a footgun. But even the issues author acknowledges, in the issue thread: Realistically: this is a pretty rare problem, and documenting it is probably a fine solution. This is probably going to cost someone else a couple of days of debugging every couple of years > It has wasted thousands of hours of people's time, either debugging the problems, or debating what to do about it. Source?
  • MrBuddyCasino 3y ago
    Unpopular opinion: Neither Go's os.Setenv nor Rust's std::env::set_var() should exist. I was pleased to find that Java only has System.getEnv(), but not a setter.
    • usrbinbash 3y ago
      That is an unpopular opinion for the simple reason that some programs do in fact need to set envvars, particularly programs that will start child processes.
      • MrBuddyCasino 3y ago
        That is still possible using the java.lang.ProcessBuilder API: you can launch a child process and give it a modified environment, but just at launch time. This side-steps the issue.
• Galanwe 3y ago
  I don't quite get why this is a problem per se. I mean, the environment is just a chunk of memory made available to the process by the OS. It is no more, no less thread safe than any other chunk of memory. Why would the libc need to protect it more than any other memory location?
• fooker 3y ago
  Changing any kind of global state is fundamentally not thread safe. Sure you could use locks, stop the world, etc, but there is no way you can ensure that all the data and information you had derived from the old state is going to be valid. A better solution is to not rely on global state like this.
• larschdk 3y ago
  What is the use case for a mutable environment past initialization? It seems like a complicated and error prone thing to be using no matter if it is thread safe or not. You can set up your own environment before you launch threads, and you can launch child processes with a different environment from the current process without modifying your own. If you fork, you can modify the environment in the child without affecting the parent until you exec. And even setenv() if it was reentrant and couldn't cause crashes, it wouldn't be thread safe, since threads share the environment and could get their environment changed under their feet.
• 1over137 3y ago
  >We should apparently read every function's specification carefully, not use software written by others, and not use threads. These are unrealistic assumptions in modern software. The first of the three listed items you should certainly do. I hope this author is not writing medical software, or anything important.
• faiD9Eet 3y ago
  I am trying to make sense of the argument of pushing configuration into a library: * if the library is just a dependency, the Linux loader will set it up. It will have the same environment as the other libraries and as the main program. * if the library is set up by dlopen(), there is no way to provide an environment pointer Altering the global environment variable for child processes makes no sense, for execve() accepts an char* envp[] . So I guess we need to talk about issues with a specific use case of dlopen()
  • planede 3y ago
    Maybe the dlopen issue could be hacked around by dlmopen and injecting getenv and setenv symbols that access a different environment variable list than the application's.
• devnonymous 3y ago
  Everytime I hear someone lament about something not being thread safe, what I actually hear is - I want this shared invariant global state to be modifiable but it isn't. Which makes me ask the question why would you want that ? The process environment should not be the mechanism for threads to communicate with each other.
• sebstefan 3y ago
  >The argument is that the specification clearly documents that setenv() cannot be used with threads. Therefore, if someone does this, the crashes are their fault. Oh, C is taking the php approach :)
  • Ayesh 3y ago
    In fairness to PHP, almost every PHP 7+ version from the 5-6 years had things deprecated especially to "cure" these things.
• fsckboy 3y ago
  the env is unix. It's not C's job to fix. Keep studying it till you understand that. and don't forget, you are using unix because it defeated all the other options, because it was better and they were worse, so also keep studying till you understand why that is too. then this problem with env will fix itself. unix gives you tools to handle threads. C gives you tools to handle threads. Learn them, use them.
• jeroenhd 3y ago
  On Windows you can use GetEnvironmentVariable/SetEnvironmentVariable (on XP and later), which do implement some locking and doesn't run into this issue because GetEnvironmentVariable copies the data out into a caller-supplied buffer. getenv_s was a nice effort, but it failed. I don't really understand why other languages such as Go and Rust decided to call the weird POSIX API rather than implementing their own API, which matches the semantics they expect. In cross platform C you'll be stuck with the outdated POSIX API design, but there's no reason why other languages should accept those same limitations. We're not running on PDP-11s anymore. You can afford a thread-safe hash map in your standard library. Ignore the limitations of the old C library. Twenty years ago, Microsoft released a better API, keep the crashy old API with tons of deprecation warnings (hell, add a compiler flag --enable-broken-c-api-designs) and just provide new APIs that are actually usable in modern programming environments.
  • pjmlp 3y ago
    Because as languages born from UNIX culture, the community usually sees POSIX everywhere, and then there are the other OSes. Have some fun reading on how Go handles files.
    • jeroenhd 3y ago
      Go likes to ignore edge cases ("all file names are UTF-8 and if they aren't then we'll just pretend they are") to make it easier to write code, so I'm not very surprised that it got caught in a POSIX related crash here. It's hard to tell if Microsoft altered the source code since, but the leaked XP source code (https://github.com/tongzx/nt5src/blob/master/Source/XPSP1/NT/net/sockets/winsock2/ws2_32/src/addrinfo.cpp#L1064 https://github.com/tongzx/nt5src/blob/master/Source/XPSP1/NT...) doesn't seem to do any getenv() calls for DNS lookups. The specific bug that started all this nonsense only triggers on (specific) Unix implementations. Unfortunately, Go opts to call the POSIX methods rather than GetEnvironmentVariable/SetEnvironmentVariable on Windows, so I suppose it's still possible that somewhere in the chain this bug gets triggered by Go code.
      • pjmlp 3y ago
• masklinn 3y ago
  Doesn’t the essay contradict itself? It states that glibc “never free[s] environment variables], but then goes on to state > [in glibc] if a thread calling setenv() needs to resize the array of pointers, it copies the values to a new array and frees the previous one Since envvars cause crash under glibc, I assume the initial assertion is incorrect.
  • Tobu 3y ago
    There are two level of pointers: the environment block points to an array of pointers to C strings, this higher-level pointer can be updated and the previous one freed, which is a problem when it is being iterated on (which getenv does). The C strings themselves aren't freed by glibc, though some applications do modify them in place.
    • masklinn 3y ago
      OIC, so because glibc doesn't hold the setenv lock during lookup a concurrent getenv can break, but once getenv has found an entry that entry is guaranteed to be valid (unless the application plays silly bugger) because setenv will not free the individual entries.
• tsukikage 3y ago
  Hot take: if a crash can be made to disappear just by adding a mutex inside setenv(), this means code reading the environment is racing with code writing to it, and in this situation adding a mutex inside setenv() will generally make things worse instead of better: it may hide the immediate symptom, but the underlying race between reading and writing to the environment remains, your program will behave differently each run depending on who wins the race (with potentially catastrophic results depending on what the writes are doing), and the cause will be much harder to debug from cold due to the lack of a smoking gun pointing at environment manipulation. The multithreaded program needs to be restructured so that the parts that communicate via the environment are properly serialised with respect to each other, just as would be needed for any other communication via global state and/or access to a shared resource. This has to happen at a higher level than the individual getenv/setenv calls: entire blocks of logic containing the calls need to be made atomic (or otherwise refactored; perhaps you could do all the environment writes before spawning any threads) so that no other thread can blow away the environment contents in between the code that sets it up for some purpose and the code that implements that purpose; and once this is properly done, the individual calls themselves do not need further protection.
  • hddqsb 3y ago
    Sure, some applications might require custom higher-level synchronisation, but it's still important for getenv/setenv to be thread-safe (i.e. not crash): - The race might be irrelevant (e.g. simultaneous calls that access different variables are fine). - The application author might not have complete control over all calls to getenv/setenv (e.g. if using a third-party library).
• dark-star 3y ago
  If you're doing setenv in multiple threads in parallel and can't afford the overhead of wrapping it in a mutex or whatever, then you're clearly doing something wrong...
  • kukkamario 3y ago
    That isn't enough. You'd need to wrap getenv calls as well and that can't be done as there are hidden getenv calls in stdlib implementation. There isn't a trivial fix.
  • tsukikage 3y ago
    if you're doing setenv in multiple threads in parallel, you clearly don't actually care about what state the environment is in at any given time, and therefore a better optimisation would be to not bother with the setenv calls at all
• planede 3y ago
  So you make it thread-safe. Now what? Just because you don't get a data-race or undefined behavior, it doesn't make setenv/getenv usable across threads without any synchronization anyway. My take on it is that global mutable state is owned by the application, library code should never ever mutate it. Applies to the environment variables, stdout/stderr, locale. The application must ensure that when these are mutated they are not read concurrently by an other thread. As external libraries rarely document the exact conditions when they read environment variables, the best is to only update the environment when no other thread is running. The absolute best is to avoid mutating it altogether.
• lifthrasiir 3y ago
  > My understanding is the people responsible for the Unix POSIX standards did not like the design of these functions, so they refused to implement them. And herein lies the actual issue: C has a sh*tton of API issues in its standard library, and people really want to fix as many of them as possible whenever possible, but doing so will destabilize the standard so most of them won't ever be accepted. In the case of Annex K many clearly felt that the size-restricted API alone is not enough, because it is still easy to desynchronize the buffer and the allocated size, and it's a good point if we ignore an obvious counterpoint of the lack of safe `getenv` alternatives in the standard library at all... I wonder about the alternative universe where we have two distinct standards for the C language and C standard library so that the library standard is much easier to fix and adapt.
• FounderBurr 3y ago
  So much cringe, I can’t believe this wasn’t written as satire.
• ChrisRR 3y ago
  Lots of C isn't thread safe, and that's the point. It's supposed to be close to the metal, and it's supposed to be small. If you want to add additional functionality then that's on the user/library. If we add thread safe/memory safe versions of every function, then we're moving away from C and into Rust territory. C is a powerful language, but it's not a language designed to hold your hand. Edit: Especially in this case where the environment is maintained by the OS, that puts the onus of safety on the OS to ensure that different processes can't modify and read the env simultaneously. If you're worried about reading and writing the env in different threads within the same process, then you need to reconsider your design.
  • rini17 3y ago
    We're talking about C standard library here. Which has streams (struct FILE) that are threadsafe and use locks by default - even in single-threaded programs. They could have extended the interface to access the environment too. But idk, reasons.
    • jstimpfle 3y ago
      Contrary to fread(), getenv() does not copy data. It returns a pointer to memory that might be invalidated by the following setenv(). In other words, it can't be really made threadsafe -- it's fundamentally a thread-unsafe API. What could be done is offering an alternative, thread-safe API that takes an RW mutex on get/set, and get reads to a user-provided buffer. However, that would be complicated as well because the user can't know the size of any envvar, and getting the size before reading the var is racy. So maybe there needs to be env_lock()/env_unlock() and getenv_unlocked()/setenv_unlocked(). Or a version that locks and strdups() the var. But that still leaves the problem that existing software does not use this API. And really, why would you set envvars instead of global variables? Seriously? A data structure that is a list of KEY=VALUE formatted zero-terminated string pointers? Just don't do it. Use setenv() only at startup and after fork() and before exec(), done.
      • rini17 3y ago
        Why? Since there's no other global key-value store handy in C, environment will continue to be (ab)used. Simple as that. Better than makeshift k-v store with global variables full of pitfalls and buffer overflows, at any rate. I believe existing software would gladly migrate.
• 0xbadcafebee 3y ago
  Much ado about nothing. If you have to set environment, do it at the beginning of a program, before threading. When you execute an application, pass a new environment, without setting it. There's really no need to set environment from threads. The only use case where this bug happens seems to be threaded programs that load libraries after threading has initialized, and want to configure those libraries with environment variables, that the user/parent program hasn't specified, rather than calling their APIs with specific arguments. If a library provides no way to set some option other than environment variables, their API is simply incomplete and needs fixing. An incomplete library is not a good enough reason to amend the C standard.
• dahfizz 3y ago
  > We should apparently read every function's specification carefully.... These are unrealistic assumptions in modern software. Lol
• ewst 3y ago
  why are you calling setenv in threads?
  • amelius 3y ago
    Possibly because some library they are using calls getenv.
• whalesalad 3y ago
  Environment is considered an immutable space in all the systems I build. It's set before any program code / processes are even started.
• Joker_vD 3y ago
  And to re-iterate my point from another thread on setenv, no, "just don't call setenv() after creating threads" is not a solution because even if the code you wrote may be single-threaded, your application as a whole is not composed entirely of code written by you: the moment you link against any 3rd-party library, you program can have arbitrarily many threads before it even reaches main().
  • account42 3y ago
    > And to re-iterate my point from another thread on setenv, no, "just don't call setenv() after creating threads" is not a solution Yes it is. > the moment you link against any 3rd-party library, you program can have arbitrarily many threads before it even reaches main(). So don't call setenv after you load any libraries. If that means you can't call setenv at all for your linking setup then so be it.
  • grayhatter 3y ago
    > the moment you link against any 3rd-party library, you program can have arbitrarily many threads before it even reaches main(). what?! name one library that does this?
    • aidenn0 3y ago
      I think the C++ version of the zeromq library could initialize a context in a static initializer, and zmq contexts involve creating a thread.
      • grayhatter 3y ago
        https://zeromq.org/get-started/?language=cpp# https://zeromq.org/get-started/?language=cpp# there's 5 listed for c++ which one creates threads before main()?
        • aidenn0 3y ago
          It's been a long time, so I might be wrong, but with zmqpp[1] make a global zmqpp::context and I believe it will create threads before main(). 1: https://github.com/zeromq/zmqpp https://github.com/zeromq/zmqpp
• egberts1 3y ago
  I am only more surprised that we have not inserted a new #ifdef UNSAFE_THREAD added to stdhdr.h, so we can wrap setenv(), et. al.
• Const-me 3y ago
  Interestingly, Windows developers made much better choices back in the 1990-s. GetEnvironmentStrings() API comes with the FreeEnvironmentStrings() counterpart. Whoever calls GetEnvironmentStrings() to get the entire environment is then responsible to call FreeEnvironmentStrings(), which allows thread-safe GetEnvironmentStrings() API. GetEnvironmentVariable() API is even simpler, it doesn’t return a pointer, instead it fills a caller-provided buffer.
  • NekkoDroid 3y ago
    GetEnvironmentVariable() also has a subtle problem from what I can see: You first check how big the buffer has to be and in the second call you actually fill out the buffer, which allows for the variable to change between those 2 calls (I guess it's a variant of the time-of-check time-of-use race condition). There is getenv_s that doesn't have this problem to my knowledge, but it also doesn't exactly allow you to control the allocation of the memory (how important that is in cases like this is a different question)
    • Const-me 3y ago
      It seems the getenv_s and GetEnvironmentVariable API functions are 100% equivalent; the API is only slightly different. When you don’t know the length of the variable you query, you guessed wrong on the first attempt, and other threads are changing the environment in the background, you might need 3 of even more calls to either function (passing longer output buffers) to successfully retrieve the complete variable.
      • deleted 3y ago
        [deleted]
      • NekkoDroid 3y ago
        You are right, I mixed up my functions. What I actually meant was _dupenv_s
• InfiniteRand 3y ago
  Beyond availability, anyone have issues with getenv_s?
• account42 3y ago
  > Since many libraries are configured through environment variables, a program may need to change these variables to configure the libraries it uses. This is common at application startup. This causes programs to need to call setenv(). Given this issue, it seems like libraries should also provide a way to explicitly configure any settings, and avoid using environment variables. This is the only correct solution. Even threadsafe, setenv doesn't guarantee anything about when the variable will take effect. There is no way for consumers to tell be notifided of a changed variable. For that you need guarantees from the library and at that point the library can just as well provide a better configuration interface. Keep environment variables static for the process lifetime and there are no issues.
• deleted 3y ago
  [deleted]
• cryptonector 3y ago
  I'm glad TFA mentions Solaris/Illumos' implementation, so I don't have to.
• cryptonector 3y ago
  I'm glad TFA mentions Solaris/Illumos' implementation, so I don't have to. Java doesn't allow one to set environment variables for the running process, but it does allow setting env vars for processes being spawned. It would be better if all C libraries did what Illumos' does.
• kibwen 3y ago
  Eyra is a new implementation of libc in Rust that addresses this in its default configuration: "Eyra solves this by having setenv etc. just leak the old memory. That ensures that it stays valid for as long as any thread needs it. Granted, leaking isn't great, and Eyra makes it configurable with the threadsafe-setenv cargo feature, so it can be disabled in favor of the thread-unsafe implementation. " https://blog.sunfishcode.online/eyra-does-the-impossible/ https://blog.sunfishcode.online/eyra-does-the-impossible/
  • deleted 3y ago
    [deleted]
  • cryptonector 3y ago
    It was never impossible. As TFA notes, Solaris/Illumos got it right 15 years ago.
• cryptonector 3y ago
  > First, we can make a thread-safe implementation, like Illumos/Solaris. This has some limitations: it leaks memory in setenv(), and is still unsafe if a program uses putenv() or the environ variable. No, `putenv()` in Solaris/Illumos is thread-safe too. And the program should NOT write to `environ`, but as it happens Solaris/Illumos allows that too (but it can cause a lock to be taken in `getenv()`.
• cryptonector 3y ago
  > First, we can make a thread-safe implementation, like Illumos/Solaris. This has some limitations: it leaks memory in setenv(), and is still unsafe if a program uses putenv() or the environ variable. No, `putenv()` in Solaris/Illumos is thread-safe too. And the program should NOT write to `environ`, but as it happens Solaris/Illumos allows that too (but it can cause a lock to be taken in `getenv()`. > Add a function to copy one single environment variable to a user-specified buffer, similar to getenv_s(). Because Windows has one? Fine, but adding a version that allocates the copy would be more convenient. BTW, HN is having problems that are leading to dup comments. I get "We're having some trouble serving your request. Sorry!", but the action happens anyways.
• StillBored 3y ago
  The problem isn't setenv() so much as getenv returns raw pointers to the data structure that is potentially being manipulated. But this is/was strictly a glibc/Linux bug because maintainers in the past didn't want to improve the situation (read add locks which weren't 100% reliable) nor add thread safe version of the calls: ex getenv_s/getenv_r as pretty much every other POSIX compliant system has done. And so the situation I hit many years ago was a proprietary library doing setenv's before fork() has now been fixed (and me calling those routines from multiple threads), and setenv() on linux/glibc is now working if it's built with locking: https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/setenv.c;h=cc71287fcc2726701ac13cbfc402979e06a0d29d;hb=HEAD#l133 https://sourceware.org/git/?p=glibc.git;a=blob;f=stdlib/sete... So the remaining issue is making the environment thread safe, which means adding a getenv_r/s call and assuring its being used everywhere, which is probably a more complex problem than tossing the setenv() lock. But then in the setenv/fork case the forked process is a crapshoot whether it gets the "right" environment. In my case above it didn't really matter because the library was doing the equivalent of `export YOURACHILD=1` so the value wasn't being changed from invocation to invocation. But there are dozens and dozens of other similar gochas in the spec, where error conditions or threading races exist and aren't noticeable until one understands how it is being implemented. There isn't a way to fix it with the library itself because many of the calls need more stored context space (ala win32 handles). So one ends up doing things like building serialization locks into the application to assure certain subsets of the posix/c/etc libraries aren't being called in parallel. (in this case getenv/setenv/fork).
• dpc_01234 3y ago
  We need to fork libc/posix, name it like `libc2` etc. fix versioning and start making changes, while allowing legacy software deal with the old cruft. It never gonna happen if we're waiting for some committee to deal with it, because they will be too afraid of breaking backward compatibility.
• leoh 3y ago
  Yeah, it’s lame. Linters and static analyzers should probably warn in addition to updating as much living documentation as possible. And, idk, suggest something like a method with a singleton mutex for getting/setting?
• worik 3y ago
  > We should apparently read every function's specification carefully, Yes. You should.
• zare_st 3y ago
  I don't see the problem here. I don't see why you need setenv. Please tell me why it's used in conjunction with pthreads. I think whoever is doing this is designing their software under wrong assumptions. Can you read/write to same fd socket across threads? No? So what's the issue then?
• Groxx 3y ago
  Is anyone aware of a language that simply does not have globals like this? Or globals at all? The more I deal with other people's code, the more I want one. Obviously there are some semantic-globals (ports, env, main thread, etc) that are unavoidable, but we have a way to deal with that: dependency injection. Allow it in main, everything else has zero access unless it is given the instance representing it. Obviously that would be pretty painful in practice without some boilerplate-reducing tools, but... would it be worse in aggregate? Or would having real control over all this at last pay off? I'm quite curious.
• andrewla 3y ago
  A lot of comments in here seem to be saying that "setenv is rarely used, so we can make setenv threadsafe even if it is costly", but I think that's missing the point. The thread-safety is in the operators getenv/setenv (and putenv and unsetenv). Thread safety has to apply to all operators, and the functionality of getenv() (which is by far the most commonly used of these operators) is what has to be fixed. You simply cannot make setenv() thread-safe so long as getenv() has its current interface. You need to make getenv() safe first; ideally with a getenv_r() call that fills in a user-supplied buffer. From that point making the rest of the calls thread safe is trivial.
• jcalvinowens 3y ago
  This is silly, setenv() isn't reentrant for the same reason that getopt() isn't reentrant: there's no valid reason to use it except at the very beginning of the program. The most common misuse I see is changing env before forking a child: nobody has to do that, execve() lets you pass arbitrary envp to the new process without changing yours. If you need to change env in threaded tests... frankly I think there was probably a better way to do whatever you're doing, but you can just declare a global lock and use it. I bet you could even LD_PRELOAD a custom setenv() that uses your lock. Nobody is pointing at concrete problems outside of Rust. Rust is just wrong here, sorry, the manpage has said this for a long time: > POSIX.1 does not require setenv() or unsetenv() to be reentrant. I think a more intellectually honest version of this article would have been "POSIX should have made setenv() reentrant", not "C is buggy": it's not buggy, it obviously complies with the standard. There's nothing to "fix", he wants to change the standard.
  • kibwen 3y ago
    > the manpage has said this for a long time Nobody is disputing what the manpage says. What people are arguing is that the specification should be improved so as to no longer say that. Please stop quoting the Posix docs, as it merely broadcasts that one has missed the point here. Documenting the behavior does not automatically excuse the behavior. As for Rust, the Rust docs make it clear that the underlying mechanism is fraught. Rust is well-acquainted with trying to find satisfactory solutions to the unhelpful and nonsensical tech stack that has been foisted upon the world by decades of worse-is-better laziness. And even if Rust were to mark std::env::set_var as unsafe, that doesn't magically fix anything; the underlying mechanism is broken, and actually fixing it is beyond Rust's control. Only the platforms can fix it.
    • jcalvinowens 3y ago
      > Documenting the behavior does not automatically excuse the behavior. It's a standard, and I'm citing the standard. The non-reentrant behavior doesn't need to be "excused", it is correct! If a microcontroller used a different bit than you expected it to for something, would the documentation need to be "excused" for "disagreeing" with you? That's how absurd what you're saying here sounds. > What people are arguing is that the specification should be improved That would be more reasonable, but that's not the argument. They're saying the standard is wrong. Standards can't be wrong, they're tautological. All that can be wrong is the programmer's understanding of them.
• JohnFen 3y ago
  getenv/setenv is also part of the library, not the language. If it presents a problem for a particular program, it's easy enough to implement your own variety that behaves as you need, or to wrap the library call in something that provides the needed thread safety. This seems like a bit of a tempest in a teapot to me.
• nwmcsween 3y ago
  This is the difference between Rust and C like languages, one optimizes for the usual case and has a simple implementation and the other goes to great lengths to be correct in corner cases at the cost of a lot of complexity.
• nwmcsween 3y ago
  Can this be renamed "setenv() is not webscale"?