Document possible panics for std::env::{set_var, unset_var}
#29298
Conversation
|
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
|
The rules for both posix and windows do seem to include that the name must not contain In reality this API panics on any OS error from the operation. It's a bit tricky to document with certainty since the number of OSes that Rust supports will be growing. |
|
Wow I'm surprised by this, I had no idea this could happen! In retrospect it makes sense, however. I agree with @bluss that we probably don't want to guarantee that it panics in these situations, but rather indicate that it may panic. It may also be worth noting that if there is a nul byte on Unix (and maybe on Windows?) a panic will happen as well. The last failure mode that I know of is OOM, but I think that's fine to not document. |
|
For anyone testing this locally as well, turns out on Unix a |
Previously the documentation suggested that the documentation about the panics are guarantees.
|
@alexcrichton Changed it to "may panic". Is it better like this? |
As discovered in rust-lang#29298, `env::set_var("", "")` will panic, but it turns out that it *also* deadlocks on Unix systems. This happens because if a panic happens while holding the environment lock, we then go try to read RUST_BACKTRACE, grabbing the environment lock, causing a deadlock. Specifically, the changes made here are: * The environment lock is pushed into `std::sys` instead of `std::env`. This also only puts it in the Unix implementation, not Windows where the functions are already threadsafe. * The `std::sys` implementation now returns `io::Result` so panics are explicitly at the `std::env` level. The panic messages have also been improved in these situations.
|
@bors: rollup |
As discovered in #29298, `env::set_var("", "")` will panic, but it turns out that it *also* deadlocks on Unix systems. This happens because if a panic happens while holding the environment lock, we then go try to read RUST_BACKTRACE, grabbing the environment lock, causing a deadlock. Specifically, the changes made here are: * The environment lock is pushed into `std::sys` instead of `std::env`. This also only puts it in the Unix implementation, not Windows where the functions are already threadsafe. * The `std::sys` implementation now returns `io::Result` so panics are explicitly at the `std::env` level.
No description provided.