Better solution for getting local offset on unix

[tmccombs]

Given that #293 is closed, but in such a way that UtcOffset::local_offset_at and current_local_offset are essentially useless on unix without also enabling the unsound_local_offset config in rustc flags. So I'm creating a new issue to track a solution that makes getting the local offset usable on unix again.

Some possible ways this could be solved are:

1. Re-implement determining the offset, similar to what datetime_r does in pure rust. This will probably require reading zoneinfo files (see tzfile(5) man page).
2. Hook into the lock on the environment variable in std somehow, this probably requires exposing an api for that from std.

[jhpratt]

There is an ongoing discussion on IRLO for how this can be achieved soundly. Exposing the environment lock has previously been ruled out.

[tmccombs]

Forgive my ignorance, what is IRLO?

[jhpratt]

Not an issue! IRLO is internals.rust-lang.org. You may also see URLO mentioned in some places, which is users.rust-lang.org.

[tmccombs]

For reference the aforementioned discussion is at https://internals.rust-lang.org/t/synchronized-ffi-access-to-posix-environment-variable-functions/15475

I did a little digging into 1. For linux specifically, I don't think it would be that difficult. The zoneinfo format isn't that complicated, and the tzfile crate already has an implementation of parsing it in rust (although that crate does depend on chrono). And reading the TZ environment doesn't look too complicated either. Unfortunately, according to this it looks like the zoneinfo format, location of zoneinfo files, and even the interpretaion of the :characters form of the TZ variable are implementation defined. But maybe most unices follow the same pattern? Or is it different on *BSDs and Mac?

This is probably also related to #193.

On a sidenote, I'm amazed that c/posix doesn't have any function that can get the local time or offset independently of the TZ environment variable. AFAICT the only ways to get the offset for a timezone other than the current one is to set the TZ variable with setenv or similar before calling localtime_r, or parsing zoneinfo files yourself, and the former isn't safe to do in a multi-threaded situation.

[jhpratt]

All the zoneinfo stuff is #193 and I don't believe that it's related to this issue in any way. I could be wrong, though: the time crate is my first foray into syscall-level APIs.

The Unix-provided method for getting the local offset is quite odd, admittedly. Unfortunately at this point there's near certainty that it won't change. That, plus the wider ecosystem impact, is why there is discussion regarding a proper solution upstream in stdlib.

[tmccombs]

I don't believe that it's related to this issue in any way. I could be wrong, though: the time crate is my first foray into syscall-level APIs

The timezone part of datetime_r doesn't involve a syscall. As I understand it tzset which may be called by localtime_r looks at the TZ environment variable, and depending on the value, may parse zoneinfo files to figure out the values for tzname (name of timezone and dst timezone), daylight (whether currently daylight savings), and timezone, which contains the UTC offset in seconds. Then localtime_r uses that information to get get a local time from a unix timestamp. The kernel isn't involved at all (although it is in getting the current unix epoch). So, to replace localtime_r we need to parse zoneinfo files, at least on linux.

[jhpratt]

After some digging around I think I know where you were going with this. I can read the /etc/localtime file, parse it, and then do #193-related things to actually figure out the offset at the given point in time. Reading the /etc/localtime file could be replaced by calling tzset and reading the environment variable, but that runs into the same issue that already exists.

No idea how widely supported this is, but it works on at least Fedora (what I use) and presumably all of Linux. If this works then it's probably worth breaking this out into a separate crate

[jhpratt]

I don't believe this is applicable to Unix in general (please let me know if it is!), but I plan on using /proc/self/task on Linux to determine if multiple threads are running. If there are not, the call to localtime_r is sound. It's a band-aid solution but it's better than the status quo.

Plan on the change this weekend with a prompt release if it works.

[jhpratt]

^^ I have just released this as part of time 0.3.5. Linux users rejoice! Mac users…not so much.

[Morganamilo]

What about spawning a new process to call the function? That would guarintee there's only one thread. The crate could also cache the function so it's fast after the first call.

[jhpratt]

What would that process be executing, exactly?

Cacheing the value is a no-go as it could change during execution of the program.

[Morganamilo]

What would that process be executing, exactly?

Itself!

If the process were to:

• fork to ensure one thread is running
• then try get an env var though std to ensure we can read the env
• then call the c function
• send the value back through a pipe

I think that could work?

[jhpratt]

You can't call a library as if it's a binary — that just doesn't make sense.

[Morganamilo]

Fork duplicates the process, not calls a binary.

[tmccombs]

fork() is itself not safe if there are multiple threads, unless it immediately calls execve or similar. One reason being that another thread could hold a lock, and when the fork happens in the new process that lock will never be released.

[Morganamilo]

Yes that's the poing of the step "then try get an env var though std to ensure we can read the env".

But now I look, std has no try functions for the environment which makes this not possble.

[AngelicosPhosphoros]

Is it possible to get timezone information from files?
https://man7.org/linux/man-pages/man3/tzset.3.html#FILES

We can cache result in OnceCell then it should be fast.

[Morganamilo]

If you want a real hack though that probably shouldn't be implemented:

The internal code for Command::spawn() locks the env before forking so that it's safe to access in the fork.
You could in theory register this as a pre exec hook to run right after the fork and effectively get a lock to the env.

And now I'm thinking the actual c function is probably not safe to run in the fork so never mind.

[jhpratt]

While not perfect, obtaining the local offset is in the best state possible for the time being. The time crate is relying on the num_threads crate for Unix systems, which currently has support for Linux, FreeBSD, and MacOS.

Because the functionality has been made available again when soundness can be guaranteed for the three most widely used operating systems, I'm going to go ahead and close this. This isn't a tracking issue for the soundness concerns of environment variables in general, after all.

[tmccombs]

Limiting the functionality to single-threaded applications is still pretty crippling though. Getting the current offset before spawning additional threads isn't necessarily an option, because the offset can be different depending on the timestamp (especially in regions with daylight savings), so it may be necessary to get the local time for a different timestamp after multiple threads have started.

The only really good solution I see to this is to stop using localtime_r, and have a pure-rust implementation of determining the offset. There are a couple of existing crates (libtzfile and zoneinfo_compiled) that can parse TZIF files, which is what linux at least uses for timezone files, however, afaict they only support using the transitions, and don't support parsing the posix TZ string used in v2 and above, which in practice is probably only important for times in the far future.

[jhpratt]

Keep in mind that the soundness issue in in std, not time. As soon as std::env::set_var is deprecated, I will restore functionality to what it previously was. It seems excessive on my end to parse a slew of time zone information for this when the operating system provides a quick, simple way to obtain it already. All that's necessary is a deprecation in std.

There is consensus on deprecating std::env::set_var, by the way. It's only a matter of what will be present as a replacement.

[tmccombs]

when the operating system provides a quick, simple way to obtain it already.

Maybe a little pedantic, but it is libc that provides that, not the operating system.

[jhpratt]

Yeah, you're right but it doesn't much matter either way.

[tmccombs]

https://github.com/x-hgg-x/tz-rsis an implementation of localtime in pure rust.

[jhpratt]

Thanks. I was made aware of that crate a few days ago. I just need the time to review the open PRs before I can take a look at the code there. My backlog is growing to say the least.

[tmccombs]

One rather significant case where this is still an issue is unit tests. By default, cargo runs tests with multiple threads, so even if your application only needs to get the local offset while single threaded, tests that use that function will fail.

See https://github.com/sharkdp/fd/pull/1190/files#diff-1fdc06e038afd6b916c6f6cf1db96920532fb43d1b66f453e744e89fb78f05b9R26

[jhpratt]

It's possible to pass --test-threads=1 to the test binary. That's what I do for this crate.

[tmccombs]

Yes. And then the tests run slower. Which is especially unfortunate if you only need it for one test. And you have to communicate that that is necessary for tests to pass.

[jhpratt]

I mean, the whole situation is unfortunate. But it's the reality of libc for you. There have been ideas thrown around before to be able to annotate a given test to run serially, but they have never gone anywhere to my knowledge.