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.

It's a library call, not a system call. You're right though, they could implement locking.

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.

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.

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.

That won’t help. The APi is broken. It returns a pointer.

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.

Call getenv() in a loop and you run out of memory.

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.

And how would you free it? The current posix API doesn't have any way to reliably free the result returned by `getenv`.

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.

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.

> 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.

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

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.

I don’t see how you can make:

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

Thread safe

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().

Requiring you to hold a mutex to safely call the function is an API change

I was not suggesting the caller hold a mutex, but rather getenv(), which eould be transparent to the caller.

Don’t you need C++11?

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.

It should never use locks. Solaris/Illumos' getenv() is lock-less. Every C library should copy that pattern.

I wonder if you could get around this by giving each thread its own environment context, and synchronizing them, asynchronously.

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.

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.

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.