test_env_get_set_multithreaded fails with Miri on Windows

[RalfJung]

  0.117855   error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `unnamed-4` and (2) non-atomic read on thread `unnamed-3` at alloc158665+0x8. (2) just happened here
  0.000062      --> /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/sys/pal/windows/os.rs:299:28
  0.000011       |
  0.000008   299 |         |buf, sz| unsafe { c::GetEnvironmentVariableW(k.as_ptr(), buf, sz) },
  0.000006       |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Data race detected between (1) non-atomic write on thread `unnamed-4` and (2) non-atomic read on thread `unnamed-3` at alloc158665+0x8. (2) just happened here
  0.000008       |
  0.000006   help: and (1) occurred earlier here
  0.000007      --> library/std/tests/env.rs:156:13
  0.000007       |
  0.000006   156 |             set_var("foo", "bar");
  0.000007       |             ^^^^^^^^^^^^^^^^^^^^^
  0.000009       = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  0.000008       = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
  0.000006       = note: BACKTRACE (of the first span) on thread `unnamed-3`:
  0.000007       = note: inside closure at /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/sys/pal/windows/os.rs:299:28: 299:75
  0.000007       = note: inside `std::sys::pal::windows::fill_utf16_buf::<{closure@std::sys::pal::windows::os::getenv::{closure#0}}, for<'a> fn(&'a [u16]) -> std::ffi::OsString {<std::ffi::OsString as std::os::windows::ffi::OsStringExt>::from_wide}, std::ffi::OsString>` at /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/sys/pal/windows/mod.rs:260:27: 260:76
  0.000007       = note: inside `std::sys::pal::windows::os::getenv` at /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/sys/pal/windows/os.rs:298:5: 301:6
  0.000006       = note: inside `std::env::_var_os` at /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/env.rs:271:5: 271:24
  0.000007       = note: inside `std::env::var_os::<&str>` at /home/runner/work/miri-test-libstd/miri-test-libstd/rust-src-patched/library/std/src/env.rs:267:5: 267:26
  0.000007   note: inside closure
  0.000007      --> library/std/tests/env.rs:150:21
  0.000007       |
  0.000006   150 |             let _ = var_os("foo");
  0.000007       |                     ^^^^^^^^^^^^^
  0.000007   
  0.000007   note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
  0.000006   
  0.006593   error: aborting due to 1 previous error
  0.000035   
  0.005739   test test_env_get_set_multithreaded ... error: test failed, to rerun pass `--test env`

Now... I seem to remember something about the Windows API actually being thread-safe? So this is probably a bug in Miri, which not report data races from concurrent environment accesses on Windows? @ChrisDenton is that right?

[ChrisDenton]

Yes GetEnvironmentVariableW and SetEnvironmentVariableW are thread safe. Windows API functions are unless otherwise noted.

[RalfJung]

Does Windows guarantee that if a thread sees an environment variable set by another thread, then there is a happens-before relationship between these operations and so other shared state can be accessed without data races? If yes, does this require one thread to read the same variable that another thread set, or does any kind of access to the environment induce such synchronization?

I am inclined to just make accesses to the environment basically the equivalent of relaxed atomics, and not induce any synchronization.

[ChrisDenton]

I know it uses a critical section when updating the environment. However, I'm not aware of any stated guarantees and searching the docs didn't turn up anything relevant.