5 ms·
This is actually not that hard to fix. Getenv() could keep several copies of the value around: one internal copy protected by a mutex, that it never returns, a
by tsimionescu 2y ago
This is actually not that hard to fix.
Getenv() could keep several copies of the value around: one internal copy protected by a mutex, that it never returns, and one copy per thread that it stores in thread local storage. When you call getenv(), it locks the mutex, checks if the current thread's value exists, populates it from the internal copy if not, and returns it. It will also install a new setenv-specific signal handler on this thread and store info about this thread having a copy.
Setenv() will then take the same mutex as getenv(), check if the internal copy is different from the new value; if it is, it will modify the internal copy, modify the local thread's copy if that has one, and then signal each other thread in the process that has a copy in TLS. The setenv signal handler will modify the local copy that thread holds.
It's gonna be slow for a large multi-threaded program, but since setenv() used to corrupt memory for such programs, they probably don't care. And for single-threaded programs, or even for programs that don't access getenv()/setenv() on multiple threads, there should be no extra overhead other than the mutex and the bookkeeping.
The only issues that would remain are programs which send the pointer they get from getenv() to other threads without ensuring locking access, and programs which rely on modifying the pointer from getenv() directly as a way to set an env var, and expect this to be visible across threads. Those are just hopelessly broken and can't use the same API - but aren't more broken then they are today.
Of course, in addition to this complex work to make the old API (mostly) thread safe, it should also offer a new API that simply returns a copy every time, doesn't promise to show modifications to your copy when setenv() gets called (you need to call getenv() again), and puts the onus on you to free that copy explicitly.
- o11c 2y agoIf you only consider `getenv`/`setenv` there are indeed many solutions, but it's not that simple. You also need to consider `putenv` (not that nasty, you just need to treat it like initial environment, which means you can't use a single range check) and accessing the `environ` variable directly (nasty). Your particular solution doesn't work because people expect `getenv` to be async-signal-safe, which means you shouldn't be allocating memory. Hmm ... doing an incref-like operation during `getenv` for a previously `setenv`ed variable that hasn't yet been accessed in this thread would be fine ... clear those refs during calls we know indicate knowledge refreshes ...
- oguz-ismail 2y ago>`putenv` (not that nasty, It's equally nasty. POSIX requires that the argument to `putenv()' not be copied, so it's not very different from assigning to `environ' directly.
- gpderetta 2y ago> accessing the `environ` variable directly (nasty). "easy": protect the page containing environ and handle the mutation from the signal handler. /s of course.
- fch42 2y ago"mutating" there involves the need to (re)allocate memory. To do so in a signal handler is hard ... because memory allocators are, while threadsafe, not async-signal-safe. You can't make a hard problem easy by asserting dependence on another (unsolved) hard problem. Btw, you can _also_ substitute libc's setenv/getenv/putenv with your own (locking) implementations, courtesy preload and all the funky features of ELF symbol resolution. Actually easy. But impossible if you link against static code using it (go ... away). Hmm. easy ? impossible ? damn this grey world. Gimme some color.
- alkonaut 2y agoThere has to be some sort of nuance regarding why this seemingly simple fix hasn't been made yet. Changing from crashing to blocking doesn't seem like a big breaking change.
- account42 2y agoBecause it doesn't actually solve anything: You're still replacing whatever getenv returned from under the nose the program code - if that happens in another thread or in a signal handler in the same thread doesn't make any difference. And that's before you even get to the `extern char *environ` global.
- xxs 2y agoB/c you never need setenv outside a single threaded command line utilities, and even then it's questionable.
- throwaway2037 2y agoSomeone above mentioned getenv_r(). I needed to Google about it. It is not impl'd by GNU GLibC (that I know). I do see it on NetBSD: https://man.netbsd.org/getenv_r.3 https://man.netbsd.org/getenv_r.3 It looks useful.
- OskarS 2y ago> it should also offer a new API that simply returns a copy every time Returning a copy isn't great (memory allocation!), the API should probably be something like: int getenv(const char *varName, char *buf, size_t bufSize, size_t *varSize); Where the caller manages the buffer and getenv writes into it (so it can e.g. be stack or statically allocated), the third argument is the size of the caller-managed buffer, then the last variable is an "out parameter" that returns the "true" length of the environment variable. Then afterwards, you can check if `*varSize > bufSize`, and if so, you need to make your buffer larger. The return value is an error code. Doing it like this, you can easily implement the "return a malloced copy" if you want to, but it also gives you the option to avoid allocation entirely. This is important for e.g. embedded or real-time applications, or anything that just likes to avoid `malloc()/free()`.