time::UtcOffset::local_offset_at() on *nix

[emkw]

Hello.

I'd like to discuss the possibility to bring back time::UtcOffset::local_offset_at() on *nix without implementing tzdb support.
Does the culprit is only the usage of tzset() function or also the use of localtime_r() function (also marked in glibc as MT-Safe env locale)?

Assuming we'll lose the possibility to change the timezone at program runtime, would it be necessary to call tzset() on each time::OffsetDateTime::now_local() invocation? Then the tzset() call can be handed to the programmer, before the threads spawn.

Then comes the concern about localtime_r() which by nature shouldn't modify the environment, but can it be unsound due to other threads modifying the environment?

[jhpratt]

Yeah, tzdb support is a completely separate issue than local_offset_at. Happy to discuss the latter here — the former has its own tracking issue; its design work and implementation has not yet started.

localtime_r doesn't modify the environment, but it returns a pointer to a tm struct that contains the data. Assuming the pointer is non-null, the data is considered initialized. We then use a couple fields from the tm struct to perform some calculations (the contents of which isn't relevant here). The issue arises where environment variables have been set in another thread, causing the environment to be moved in memory. This invalidates the pointer and the struct it backs, causing a segfault.

It's worth noting that tzset is not the issue. The only issue in any way is that Rust doesn't provide the ability to lock the environment to user code, though it can do so itself. As such any FFI usage of getenv is unsound, as a pure-Rust setenv isn't aware that something is already using that memory. Short of a pure-Rust getenv, I don't think this is possible to fix without something upstream from stdlib.

[emkw]

So at the moment unless stdlib provides a solution to run under ENV_LOCK held, restructuring the function to be env-thread-safe seems unlikely, or there may be some other possibility than localtime_r() to obtain local time offset with current codebase?

[jhpratt]

To the best of my knowledge that is in fact the situation. The code inherently requires calling localtime_r followed by using the value behind the pointer. Without some way to ensure that the value isn't moved in this short time, it is unsound. If there were a pure Rust localtime_r implementation, that would of course resolve the issue as stdlib uses locks for getenv. However I don't believe this is possible given that it's a system API that inherently requires FFI.