News for hackers. Stuff that matters.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by jrmg on Monday October 16, 2023 @09:13PM (#37909023)
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?

Re:Getaddrinfo() on glibc calls getenv(), oh boy by JoshTriplett on Monday October 16, 2023 @09:38PM (#37909278)
Re:Getaddrinfo() on glibc calls getenv(), oh boy by thayne on Monday October 16, 2023 @11:34PM (#37910248)
That would be nice. What is the likelihood this will actually make it into libc implementations and/or become part of the posix standard?
Re:Getaddrinfo() on glibc calls getenv(), oh boy by asveikau on Monday October 16, 2023 @09:18PM (#37909080)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by jeroenhd on Tuesday October 17, 2023 @06:14AM (#37912818)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by asveikau on Tuesday October 17, 2023 @11:38AM (#37916819)
> 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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by jeroenhd on Tuesday October 17, 2023 @05:05PM (#37921561)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by asveikau on Tuesday October 17, 2023 @05:36PM (#37921934)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by woodruffw on Monday October 16, 2023 @09:21PM (#37909111)
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...

[2]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/s...

Re:Getaddrinfo() on glibc calls getenv(), oh boy by YetAnotherNick on Monday October 16, 2023 @09:32PM (#37909228)
But getenv/setenv syscall could still be under lock, which I think most of the people would use. Walking over memory could be without lock and the program could see inconsistent values as is the current behaviour.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by aaronmdjones on Monday October 16, 2023 @09:33PM (#37909239)
It's a library call, not a system call. You're right though, they could implement locking.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by asveikau on Monday October 16, 2023 @09:37PM (#37909272)
I think in libc, there's a lot of stuff where an interface is kind of broken from a thread perspective, and it could be implemented better without changing the interface, but often people generally do not.

I can't think of any examples offhand, but I often think it about thread-local storage. Eg. lots of interfaces have an _r() equivalent where you provide the buffer, but many people still call the unsafe one which is broken when there are threads... In my mind, the best way to do this would be to use static thread-local storage in the non-_r() one, and have it call the _r() one ... Sure that has overhead and isn't a perfect solution, but it's better than "bad". But a lot of these old functions don't necessarily get love.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by Gibbon1 on Tuesday October 17, 2023 @01:07PM (#37918149)
An a sane world creating a thread would set a global that causes non thread safe library functions to seg fault. Or maybe calling them from two different threads causes a seg fault. But just make it really obvious you're doing bad stuff.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by asveikau on Tuesday October 17, 2023 @01:46PM (#37918760)
I think that sounds completely insane. The unsafety is an emergent property of what's done in the function, and entirely dependent on usage. If you were doing very disciplined use of an unsafe call, it's harmless.

Perhaps this would be a good feature of an assert, or something that breaks in a debugger if it's attached. But I don't think that is reasonable for production.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Monday October 16, 2023 @10:32PM (#37909799)
That won’t help. The APi is broken. It returns a pointer.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by Sprocklem on Monday October 16, 2023 @11:37PM (#37910271)
The pointer isn't guaranteed to point into `environ` directly. `getenv()` could copy the value to a thread-local, dynamically-allocated buffer while holding the lock.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by alexey-salmin on Monday October 16, 2023 @11:52PM (#37910377)
Call getenv() in a loop and you run out of memory.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by harerazer on Tuesday October 17, 2023 @12:26AM (#37910607)
Then don't do that. Of all the footguns in POSIX/C programming, having to remember to free this is really not as bad as you seem to imply.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by thayne on Tuesday October 17, 2023 @01:14AM (#37910878)
And how would you free it? The current posix API doesn't have any way to reliably free the result returned by `getenv`.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by alexey-salmin on Tuesday October 17, 2023 @04:57AM (#37912275)
You miss the point. If you have full control over when and how getenv is called, there's no issue to begin with. The problem is that you don't, as OP demonstrates. It's perfectly natural to call getaddrinfo in a loop.

We need a new API which is not broken like in NetBSD, and a multi-year migration of all core libraries to it. Well a pity it wasn't started years ago though, could've been 95% done by now.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by Sprocklem on Tuesday October 17, 2023 @12:24AM (#37910599)
I was suggesting that the buffer be invalidated by each subsequent call – like some other libc functions' internal buffers – although, as I noted in the edit this would need `getenv()` to be able to indicate errors (specifically ENOMEM). It currently cannot do this as currently described, because NULL is used to indicate an absent variable.

You could also require callers free the returned memory when they're done, but that would be another change of API.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by alexey-salmin on Tuesday October 17, 2023 @09:12AM (#37914441)
> I was suggesting that the buffer be invalidated by each subsequent call

This would break very simple single-threaded programs that e.g. print two env vars in one printf call.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Tuesday October 17, 2023 @12:58PM (#37918026)
The solution to all problems like this was decided years ago: _r

You provide the storage and free it

The problem is these non-direct uses. They each need to switch to •_r and manage the buffer, or offer _r versions themselves and sort of pass through the problem

Re:Getaddrinfo() on glibc calls getenv(), oh boy by Sprocklem on Tuesday October 17, 2023 @05:00PM (#37921496)
Of course, *_r is a better option, but the existing API is used so pervasively that it needs to be made thread-safe to actually avoid thread-unsafe code in, e.g, libraries.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Tuesday October 17, 2023 @11:44PM (#37924638)
I don’t see how you can make:

- return a pointer - the library owns the allocation - the state is global and mutable

Thread safe

Re:Getaddrinfo() on glibc calls getenv(), oh boy by Sprocklem on Wednesday October 18, 2023 @07:54AM (#37927389)
A number of libc functions return a pointer to an internal thread-local buffer, which is invalidated on subsequent calls. If the function copies the environment variable's value to such a buffer while holding the mutex controlling access to the global state, then the returned value is guaranteed to remain unaffected by other threads.

There are, however, other problems (discussed elsewhere in this thread) that complicate such an API in the context of getenv().

Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Wednesday October 18, 2023 @11:16AM (#37929884)
Requiring you to hold a mutex to safely call the function is an API change
Re:Getaddrinfo() on glibc calls getenv(), oh boy by Sprocklem on Wednesday October 18, 2023 @03:47PM (#37933817)
I was not suggesting the caller hold a mutex, but rather getenv(), which eould be transparent to the caller.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Thursday October 19, 2023 @08:37PM (#37950759)
Don’t you need C++11?
Re:Getaddrinfo() on glibc calls getenv(), oh boy by lokar on Tuesday October 17, 2023 @11:05AM (#37916283)
Libc and much of posix predate threads. There was no way to fix everything without changing the APIs, which they did in many cases but not all.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by cryptonector on Tuesday October 17, 2023 @02:22AM (#37911290)
It should never use locks. Solaris/Illumos' getenv() is lock-less. Every C library should copy that pattern.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by Retr0id on Monday October 16, 2023 @09:28PM (#37909189)
I wonder if you could get around this by giving each thread its own environment context, and synchronizing them, asynchronously.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by gpderetta on Tuesday October 17, 2023 @08:32AM (#37913951)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by tinus_hn on Tuesday October 17, 2023 @09:08AM (#37914387)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by jrmg on Monday October 16, 2023 @09:28PM (#37909184)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by raggi on Monday October 16, 2023 @09:23PM (#37909128)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by dundarious on Monday October 16, 2023 @10:29PM (#37909767)
Yes, on Linux you can even write `int main(int argc, char* argv[], char* envp[])`.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by o11c on Tuesday October 17, 2023 @12:29AM (#37910638)
`envp` is unsafe after `setenv` even in single-threaded programs though. So you really should use `environ`.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by FridgeSeal on Monday October 16, 2023 @09:21PM (#37909113)
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”.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by jrmg on Monday October 16, 2023 @09:23PM (#37909133)
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.”

Re:Getaddrinfo() on glibc calls getenv(), oh boy by jiggawatts on Monday October 16, 2023 @11:00PM (#37910023)
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...
Re:Getaddrinfo() on glibc calls getenv(), oh boy by thayne on Tuesday October 17, 2023 @01:29AM (#37910971)
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 and https://github.com/chronotope/chrono/issues/499

https://github.com/rust-lang/rust/issues/27970

https://github.com/rust-lang/rust/issues/90308

Re:Getaddrinfo() on glibc calls getenv(), oh boy by Taniwha on Monday October 16, 2023 @11:28PM (#37910215)
Yes it's worth remembering that the POSIX base came before threads became commonly available (or even had a standardised API)
Re:Getaddrinfo() on glibc calls getenv(), oh boy by thayne on Tuesday October 17, 2023 @01:30AM (#37910976)
Maybe it is time to replace the POSIX base with something that is better suited to a multi-threaded environemnt.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by FridgeSeal on Tuesday October 17, 2023 @05:29AM (#37912499)
I’ll go one further and say that maybe it’s time we had OS’/kernel/API base that isn’t just better suited, but is explicitly designed for the massively multithreaded, ludicrously fast, massively concurrent hardware we have in spades these days.

Alas I am not as OS dev, I have not the skills or understanding to not how to build that, or what this would involve, but I do think it’s clear that what we have at the moment isn’t as well suited as it could be. Io_uring / Direct-IO seem to be better suited though.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by pjmlp on Tuesday October 17, 2023 @06:08AM (#37912770)
Which is already happening on non UNIX OSes.

Even on OSes that happen to use the Linux kernel like Android, those that insist on using the NDK and pretend it is like GNU/Linux, beyond the official supported use cases, end up bumping their heads against the wall.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by wmf on Tuesday October 17, 2023 @12:05AM (#37910480)
This is fine for a 1980s monolithic program but if you use any library that reads environment variables (like, ahem, libc!) you have to treat the whole library as non-thread-safe? Or keep track of the "color" of each library function?
Re:Getaddrinfo() on glibc calls getenv(), oh boy by j16sdiz on Monday October 16, 2023 @09:44PM (#37909334)
For libc-developer, they could do better.

For the rest of us, I guess this is the best option. We can do wrappers, but, yak!

Re:Getaddrinfo() on glibc calls getenv(), oh boy by dataflow on Monday October 16, 2023 @09:30PM (#37909204)
That's why getenv_s et al were added.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by wahern on Monday October 16, 2023 @10:21PM (#37909699)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by thayne on Monday October 16, 2023 @11:25PM (#37910195)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by lazide on Tuesday October 17, 2023 @12:38AM (#37910692)
Deadlock disaster that one.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by cryptonector on Tuesday October 17, 2023 @02:20AM (#37911279)
`getenv()` never returns pointers that can be invalidated. If you `putenv()` something, you commit to never freeing it or overwriting it.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by hddqsb on Tuesday October 17, 2023 @05:14AM (#37912376)
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...).

Re:Getaddrinfo() on glibc calls getenv(), oh boy by cryptonector on Tuesday October 17, 2023 @11:31AM (#37916719)
"May", but really, it's best for `setenv()` to leak.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by jclulow on Monday October 16, 2023 @10:41PM (#37909866)
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

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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by o11c on Tuesday October 17, 2023 @12:26AM (#37910616)
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).

Re:Getaddrinfo() on glibc calls getenv(), oh boy by tjfontaine on Tuesday October 17, 2023 @12:45AM (#37910731)
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/...

Re:Getaddrinfo() on glibc calls getenv(), oh boy by hddqsb on Tuesday October 17, 2023 @05:36AM (#37912534)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by o11c on Tuesday October 17, 2023 @11:35AM (#37916786)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by forrestthewoods on Monday October 16, 2023 @10:44PM (#37909885)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by gpderetta on Tuesday October 17, 2023 @02:39AM (#37911388)
Should we also get rid of filesystems? Databases? All form of RPC?
Re:Getaddrinfo() on glibc calls getenv(), oh boy by nikanj on Tuesday October 17, 2023 @02:47AM (#37911433)
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.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by forrestthewoods on Tuesday October 17, 2023 @03:34AM (#37911719)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by bayindirh on Tuesday October 17, 2023 @03:34AM (#37911716)
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.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by gpderetta on Tuesday October 17, 2023 @04:09AM (#37911923)
They are dynamically scoped variables. Very powerful, but only a slight step above globals.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by cryptonector on Tuesday October 17, 2023 @02:21AM (#37911285)
On Solaris/Illumos `putenv()` and `getenv()` are lock-less and really fast.

Basically, if you `putenv()`, you commit to never freeing that memory.

Re:Getaddrinfo() on glibc calls getenv(), oh boy by xy2_ on Tuesday October 17, 2023 @04:08AM (#37911918)
Re:Getaddrinfo() on glibc calls getenv(), oh boy by cryptonector on Tuesday October 17, 2023 @11:31AM (#37916722)
Ay, yes. Though it could be made lock-less.
Re:Getaddrinfo() on glibc calls getenv(), oh boy by gpderetta on Tuesday October 17, 2023 @04:05AM (#37911902)
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.