Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

C stdlib isn't threadsafe and even safe Rust didn't save us

edgedb.com

Post reply on HN
Print view View original

41–50 of 370 posts

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#41

Post by cogman10 »

wmf wrote: post #14
umpalumpaaa wrote: post #9

Earlier quoted context omitted.

The man page says: > POSIX.1 does not require setenv() or unsetenv() to be reentrant. A non-reentrant function cannot be thread safe. In general (for POSIX, libc and many other libraries: if the docs do not explicitly say "this function is thread safe" they are not).

It's time to move beyond this attitude and make things safe by default. For example, Solaris has a safer version of setenv(). "It is ridiculous that this has been a known problem for so long. It has wasted thousands of hours of people's time, either debugging the problems, or debating what to do about it. We know how to fix the problem." https://www.evanjones.ca/setenv-is-not-thread-safe.html

You can't.

You could wrap setenv in a mutex, but that's not good enough. It can still be called from different processes, which means you'd need to do a more expensive and complex syncing system to make it safe.

That ballons out to other env related methods needing to honor the synchronization primitive in order for there to be a semblance of safety.

However, you still end up in a scenario where you can call

    setenv
    getenv
and that would be incorrect because between the set and the get, even with mutexes properly in place and coordinated amongst different applications, you have a race condition where your set can be overwritten by another application's set before your get can run. Now, instead of actually making these functions safe you've buried the fact that external processes (or your own threads) can mess with env state.

The solution is to stop using env as some sort of global variable and instead treat it as a constant when the application starts. Using setenv should be mostly discouraged because of these issues.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#42

Post by benatkin »

mmastrac wrote: post #5

The major takeaway from this is that Rust will be making environment setters unsafe in the next edition. With luck, this will filter down into crates that trigger these crashes ( https://github.com/alexcrichton/openssl-probe/issues/30 filed upstream in the meantime).

People get trained to ignore the ____UNSAFE_payattention__nevermindthatthisappears50timesinthisfile___ blocks and prefixes

This also shows up in web frameworks where Vue has the v-html directive and react has dangerouslySetInnerHTML. Vue definitely has it better.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#43

Post by Animats »

masklinn wrote: post #15

Previously on setenv being a terrible thing: https://www.evanjones.ca/setenv-is-not-thread-safe.html (discussion: https://news.ycombinator.com/item?id=38342642 first comment is even about it causing issues in Rust)

Yes. That's known.

Most of the rest of the problem here seems to be the development environment. They're testing on a remote machine in an Amazon data center and using Docker. This rig fails to report that a process has crashed. Then they don't have enough debug symbol info inside their container to get a backtrace. If they'd gotten a clean backtrace reported on the first failure, this would have been obvious.

Why is anyone using "setenv" anyway?

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#44

Post by wahern »

ChrisSD wrote: post #19
Thaxll wrote: post #13

Earlier quoted context omitted.

Why requiring unsafe when the std implementation could take care of the synchronisation?

It can only synchronize if everything using is Rust's functions. But that's not a given. People can use C libraries (especially libc) which won't be aware of Rust's locks. Or they could even use a high level runtime with its own locking but then they'll be distinct from Rust's locks. The only way to coordinate locking would be to do so in libc itself.

libc does do locking, but it's insufficient. The semantics of getenv/setenv/putenv just aren't safe for multi-threaded mutation, period, because the addresses are exposed. It's not really even a C language issue; were you to design a thread-safe env API, for C or Rust, it would look much different, likely relying on string copying even on reads rather than passing strings by reference (reference counted immutable strings would work, too, but is probably too heavy handed), and definitely not exposing the environ array.

The closest libc can get to MT safety is to never deallocate an environment string or an environ array. Solaris does this--if you continually add new variables with setenv it just leaks environ array memory, or if you continually overwrite a key it just leaks the old value. (IIRC, glibc is halfway there.) But even then it still requires the application to abstain from doing crazy stuff, like modifying the strings you get back from getenv. NetBSD tried adding safer interfaces, like getenv_r, but it's ultimately insufficient to meaningfully address the problem.

The right answer for safe, portable programs is to not mutate the environment once you go multi-threaded, or even better just treat process environment as immutable once you enter your main loop or otherwise finish with initial process setup. glibc could (and maybe should) fully adopt the Solaris solution (currently, IIRC, glibc leaks env strings but not environ arrays), but if applications are using the environment variable table as a global, shared, mutable key-value store, then leaking memory probably isn't what they want, either. Either way, the best solution is to stop treating it as mutable.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#45

Post by fsckboy »

Thaxll wrote: post #13
ChrisSD wrote: post #4

In the Rust std, `set_var` and `remove_var` will correctly require using an `unsafe {}` block in the next edition (2024). The documentation does now mention the safety issue but obviously it was a mistake to make these functions safe originally (albeit a mistake even higher level languages have made). https://doc.rust-lang.org/stable/std/env/fn.set_var.html There is a patch for glibc which makes `getenv` safe in more…

Why requiring unsafe when the std implementation could take care of the synchronisation?

you've gotten a lot of answers which say the same thing, but which I don't think answer your question:

synchronization methods impose various complexity and performance penalties, and single threaded applications which don't need that would pay those penalties and get no benefit.

Unix was designed around a lightweight ethos that allowed simple combining of functions by the user on the command line. See "worse is better", but tl;dr that way of doing things proved better, and that's why you find yourself confronting what it doesn't do.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#46

Post by mmastrac »

Animats wrote: post #43
masklinn wrote: post #15

Previously on setenv being a terrible thing: https://www.evanjones.ca/setenv-is-not-thread-safe.html (discussion: https://news.ycombinator.com/item?id=38342642 first comment is even about it causing issues in Rust)

Yes. That's known. Most of the rest of the problem here seems to be the development environment. They're testing on a remote machine in an Amazon data center and using Docker. This rig fails to report that a process has crashed. Then they don't have enough debug symbol info inside their container to get a backtrace. If they'd gotten a clean backtrace reported on the first failure, this would have been obvious. Why is…

Yup, it's mostly just the story and tools we used to get ourselves out of a mess that was made harder by some decisions made earlier -- the tests were running in a container with stripped symbols (we're going to ship symbols after this, no reason to over-optimize), our custom test runner failed to report process death (an oversight).

There's no reason setenv should have been called here. The `openssl-probe` library could simply return the paths to the system cert files and callers could plug those directly into the OpenSSL config.

Oversights all around and hopefully this continues to improve.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#48

Post by adrian_b »

jabl wrote: post #34
umpalumpaaa wrote: post #9

Earlier quoted context omitted.

The man page says: > POSIX.1 does not require setenv() or unsetenv() to be reentrant. A non-reentrant function cannot be thread safe. In general (for POSIX, libc and many other libraries: if the docs do not explicitly say "this function is thread safe" they are not).

> A non-reentrant function cannot be thread safe. Actually, a non-reentrant function can be thread-safe. A common example of such a function in libc being malloc().

By definition, a "reentrant function" is a function that may be invoked even when it has not returned yet from a previous invocation.

So a non-reentrant function is a function that may not be invoked again between a previous invocation and returning from that invocation.

When a function may be invoked from different threads, then it is certain that sometimes it will be invoked by a thread before returning from a previous invocation from a different thread.

Therefore any function that may be invoked from different threads must be reentrant. Otherwise the behavior of the program is unpredictable. Reentrant functions may be required even in single-thread programs, when they may be invoked recursively, or they may be invoked by signal handlers.

An implementation of "malloc" may be reentrant or it may be non-reentrant.

Old "malloc" implementations were usually non-reentrant because they used global variables for managing the heap. Such "malloc" functions could not be used in multi-threaded programs.

Modern "malloc" implementations are reentrant, either by using only thread-local storage or by using shared global variables to which some method for concurrent access is implemented, e.g. with mutual exclusion.

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#49

Post by gavinhoward »

It is weird that I got this right before Rust did.

Because I use structured concurrency, I can make it so every thread has its own environment stack. To add to a new environment, I duplicate it, add the new variable, and push the new enviroment on the stack.

Then I can use code blocks to delimit where that stack should be popped. [1]

This is all perfectly safe, no `unsafe` required, and can even extend to other things like the current working directory. [2]

IMO, Rust got this wrong 10 years ago when Leakpocalypse broke. [3]

[1]: https://git.yzena.com/Yzena/Yc/src/branch/master/tests/yao/e...

[2]: https://gavinhoward.com/2024/09/rewriting-rust-a-response/#g...

[3]: https://gavinhoward.com/2024/05/what-rust-got-wrong-on-forma...

Re: C stdlib isn't threadsafe and even safe Rust didn't save us

#50

Post by ChrisSD »

wahern wrote: post #44
ChrisSD wrote: post #19

Earlier quoted context omitted.

It can only synchronize if everything using is Rust's functions. But that's not a given. People can use C libraries (especially libc) which won't be aware of Rust's locks. Or they could even use a high level runtime with its own locking but then they'll be distinct from Rust's locks. The only way to coordinate locking would be to do so in libc itself.

libc does do locking, but it's insufficient. The semantics of getenv/setenv/putenv just aren't safe for multi-threaded mutation, period, because the addresses are exposed. It's not really even a C language issue; were you to design a thread-safe env API, for C or Rust, it would look much different, likely relying on string copying even on reads rather than passing strings by reference (reference counted immutable str…

A safe API would look a lot like Windows' GetEnvironmentVariable and SetEnvironmentVariable

https://learn.microsoft.com/en-us/windows/win32/api/winbase/...

https://learn.microsoft.com/en-us/windows/win32/api/winbase/...

Post reply on HN