7 ms·
> 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
by 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 agoUnpopular 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 agoThat 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 agoThat 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.
- ryan-c 3y agoPrograms that need to set environment variables for child processes should use `execvpe` or `execle`.
- account42 3y agoOr posix_spawn / posix_spawnp.
- badsectoracula 3y agoOr programs that rely on libraries that for some unfathomable reason expose some functionality only via environment variables without an API. looks at SDL
- account42 3y agoThere is SDL_SetHint [0] which doesn't modify the environment but instead changes the value internally to SDL only. [0] https://wiki.libsdl.org/SDL2/SDL_SetHint https://wiki.libsdl.org/SDL2/SDL_SetHint
- badsectoracula 3y agoThat is SDL2, what i had in mind is SDL1 - but also the SDL1-on-SDL2 implementation which had some SDL2-specific extras (for things like scaling).
- pajko 3y agoNope, execve() and friends ending in 'e' accept a pointer to a completely new set of environment variables, no need to do setenv. Windows has _execve() too.
- usrbinbash 3y agoThe fact that there is an alternative doesn't change the fact that a lot of software relies on the worse method to work.
- rerdavies 3y agoA lot of software doesn't modify the environment when exec-ing.
- usrbinbash 3y agoSo? A lot of software doesn't use file compression, should we remove these libraries?
- naniwaduni 3y agoIt makes it a pretty silly idea to invite people writing new programs in a new language to use that method, though.
- deleted 3y ago[deleted]
- kevincox 3y agoI think the probably is really that there are 2 times where you should be setting env vars 99% of the time. 1. Right after program startup before any threads are spawned. 2. After a fork before an exec. In both cases it can be known that no threads are running. (Ok, for 1 it can actually be non-trivial if you have code before main or if you call functions that spawn helper threads, but let's assume that you can know this). However no languages actually have ways to enforce this. So the APIs can be called at any time and are huge footguns. I think that the proposed improvement of `getenv_s` is great. It is cheap and easy to use, then software can slowly migrate off of the less safe stuff. You can imagine that if libc stopped using `getenv` internally most of this problem would be solved.
- Blikkentrekker 3y agoNo, in many cases one needs to set them interactively. Consider for instance something as simple a implementing a shell. Such a program needs to be able to set the environment based on user interaction and this change needs to show up in /proc/$pid/env.
- __david__ 3y agoWhy does a shell need its current environment to be visible in /proc/$pid/env (as opposed to just its initial environment)?
- Blikkentrekker 3y agoBecause the specification of the POSIX shell says that `export` changes the current environment of the running process, not just of any newly started processes. This is useful to recognize various processes I suppose. I have written code that scans the environment of processes to find particular processes and group them together.
- rerdavies 3y agoIf you need to set environment variables for child processes in a thread-safe manner, use execvpe or execle.
- jeroenhd 3y agoI think there are good reasons for Setenv and set_var to exist, but if they are implemented, they shouldn't be wrappers around POSIX' shitty API and implement their own environment variable system instead (one of which the initial variables are possibly initialised by a call to getenv to make them compatible). There's no reason why these languages need to restrict themselves the same way C does.
- MrBuddyCasino 3y agoThe bug in Golang was because DNS lookups interact with the C library, which looks up environment variables. As long as everything happens in Goland, there is no problem - but this is simply not good enough.
- jeroenhd 3y agoGo makes the assumption that the DNS lookups are thread-safe, but it doesn't have that guarantee (or the C library is spec-incompliant, but I doubt that). It's still something Go can fix. You can't fix C libraries loaded into Go programs (i.e. and external library calling C's setenv, or I suppose explicit FFI calls by the user), but Go can be responsible for the APIs it calls itself. That may necessitate writing a thread-safe alternative for DNS lookups, or documenting and/or adding compile time warnings that threaded programs doing DNS lookups will just crash sometimes, but the language's standard library can still make it much harder for developers to write buggy code.
- MrBuddyCasino 3y agoMy impression is that this was Golang's plan from the start - this is why they didn't want to use the C stdlib at all, issuing the Kernel syscalls directly from the Golang runtime. A good idea, but then they had to backpedal to solve issues such as DNS resolution respecting certain OS settings, and this bug is a symptom of that.
- fch42 3y agoYes, there are certain things in UNIX which _are_ part of the standard (POSIX / IEEE1003) but _aren't_ usually implemented as system calls. Name lookups (whether user identities or network resources) are the biggest chunk of these. You have a "choice" as a user/programmer here. Say, the existing name lookup interfaces in most libc implementations don't do DNS-over-HTTP (DoH); you can implement that yourself and just use the addresses returned by your library/package where the system calls ... want addresses. If you have the go stance, go all the way. Don't say "the C runtime is sh*te but I really really really want that one particular teensy tiny bit of it could someone somewhere somehow please do something to make it a little less sh*te". Legacy baggage is a burden and backwards compatibility shackles you. The C/Unix interfaces are full of this, and with the hindsight of 50 years noone today, not even "C programmers", would implement them all the same way again. But that doesn't mean their behaviour can be arbitrarily changed.
- grodriguez100 3y agoI fully agree with your unpopular opinion.
- crabbone 3y agoThis is a bizarre take... Programmers have to deal with a lot of badly written programs all of the time. You'd need this functionality to either debug a program that responds differently to different values of environment variables, or to control it, because, maybe it's the only reasonable way to do so. It's OK to say that programmers shouldn't rely on this functionality ideally, but, for practical reasons, this functionality is needed. Same happens in "pure" functional languages, for example, when you need to debug programs in such languages interactively, and struggle to create the program state that reproduces the problem, or, in some extreme cases, due to I/O being "impure" even struggle to output diagnostic information.
- deleted 3y ago[deleted]
- jeroenhd 3y ago> Or: C knows that it doesn't need fixing. People don't like APIs that can randomly crash your program while there's no good technical reason for why they should. Why not fix the problem? People like you, who have no issues with the current implementation, won't see any regressions because you're already a good citizen, and myriad other programmers whose programs do occasionally crash because of this will be helped. > So, is there a potential issue with thread safetey? Yes. Does it matter given where and under what circumstances it occurs? Not really. "The unpredictable crashes only happen very rarely" doesn't mean the crashes go away. > What kind of actual real life production code would continuously set envvars while simutaneously calling a function that tries to read the environment? The reproduction sample calls setenv in a loop so the issue can be reproduced. A single setenv anywhere in the code is enough to trigger the crash, but then you would get one of those "you need to run the program a million times to reproduce it" bug reports that gets pushed down the line.
- usrbinbash 3y ago> Why not fix the problem? Because doing so breaks backwards compatibility, simple as that. The problem isn't even that `setenv` isn't thread save. The problem is that `getenv` returns a `*char` directly into the environment memory space. Many many many programs rely on that being the case. > People like you People like me would like every software to be perfect, but that's not the world we live in, so we are forced to be pragmatic. When fixing something causes more problems by breaking backwards compatibility promises, than it prevents, then there is no good argument for a fix, and the correct approach is to say "yes, this sucks, let's document it well so people don't waste too much time on this". The setenv/getenv problem is such a case. Anyone who disagrees is free to fork glibc, implement whatever fix they think is adequate, and then try to compile the software packages found on a typical Linux server against the result. > so the issue can be reproduced. "Can be reproduced" and "is a common issue in production code" are not the same. Fact is, almost all production programs that set envvars, do so once, very early in the process lifecycle, and then never again, and so are never affected by this.
- mastax 3y agoSo why not implement the fix suggested in the article: improve the existing interface to the extent possible, and introduce a new interface which is easier to use correctly.
- xbar 3y agoWhile I am sure that thousands of hours have been spent debugging threaded setenv() attempts (and developing & discarding Annex K), it is clearly not a problem that needs a solution. Languages that compile to C need be careful not to promise thread-safe implementations of POSIX or C functions that are explicitly documented as not reliably thread-safe, including setenv(). The author seems to want to change C, and POSIX, so that Go can reliably do so.
- deleted 3y ago[deleted]
- loup-vaillant 3y agoYou are literally putting forth arguments in favour of fixing the thread safety issue, and then conclude it’s not worth the effort. It’s simple, really: we indeed rarely to `setenv()`. So it’s not a performance problem. So we can make it thread safe, and the performance impact will be negligible. In exchange for this small price, safety will increase. Sacrificing any amount of safety for a negligible improvement in performance is flat out unprofessional, and should be grounds for immediate termination in most contexts.
- DSMan195276 3y agoHow do you propose making it thread-safe? The real problem here is that `getenv()` was designed around it returning a `char *` into some read-only memory. It's a bad API if the backing data can change because the returned pointer is assumed to exist 'forever'. `setenv()` has no way to knowing where those pointers are floating around so there's no way to safely change the environment variables. The best you could do would be to leak memory every time you set new environment variables so that the old pointers don't get invalidated, and that just creates a new problem and reason not to use `setenv()` (that's arguably worse).
- cnity 3y agoHere's my proposal: Introduce a new threadsafe API (`tgetenv` or whatever) which takes _two_ `char *`s, one of which is a return buffer. This leaves allocation as a responsibility of the caller. And then you can leave the existing syscalls as they are (thread unsafe) while having a separate thread safe version.
- DSMan195276 3y agoI agree that would be the way to do it, but now we're no longer talking about simply 'fixing' the implementation of the existing API but rather introducing a new function you have to use. `setenv()` would only be safe if your program never uses `getenv()`, and calls to `getenv()` are so numerous and all over the place that for most non-trivial programs it would be hard to ensure they never happen. There's also the rub that `setenv()` is not part of the C standard, it's POSIX. I don't think the C standard would ever introduce `tgetenv()` to fix a problem it doesn't have, so non-POSIX code would have to continue to call `getenv()` since that's all that is available to them.
- somat 3y agoRight, why is the problem "changing memory in a shared memory execution model will cause corruption" and not "Why are we using such a fragile shared memory execution model in the first place."