Add macOS to local_offset_at's OS_HAS_THREAD_SAFE_ENVIRONMENT

[PaulDance]

Dear maintainers,

I know this may seem like the 1298th duplicate issue about the underlying problem at first glance, but I have seen the previous issues regarding macOS vs. local offsets and still think something may have been missed. In any case, I'm open to discussion, hence my opening this.

Indeed, the local_offset_at has a few soundness checks, one of them being OS_HAS_THREAD_SAFE_ENVIRONMENT. When true, which is rarely the case, further soundness checks are disabled.

However, macOS is not part of the list, while what I'm seeing seems to indicate it should: although FreeBSD's implementation is most probably not thread-safe and the macOS man pages do not mention it either, the actual Apple libc implementations of getenv and setenv really do seem to be thread-safe, which was added in this commit, itself included as part of macOS 10.12.

Therefore, is there still a good reason not to include macOS in the thread-safe list? I've seen segmentation faults mentioned around here and specifically for this platform, but I would be surprised it would come from the environment manipulation itself.

Thanks in advance,
Paul.

[jhpratt]

No worries! It's immediately clear that this is not the same thing brought up repeatedly :)

Without digging into this again, you're 100% correct. I previously verified it, though after someone brought up that the behavior was relatively recent I removed the opt-out. While I don't recall the timeframe when this happened, it has almost certainly been long enough now.

With that said, there is no reason to open a pull request to add an opt-out here. On one of my SSDs I have a few commits that I haven't pushed yet, one of which removes the soundness checks entirely in favor of a better solution. These commits should be pushed in the coming days when I get a new laptop set up (hence the temporary storage).

[PaulDance]

Good, thanks for the quick answer, glad to hear!

[jhpratt]

This is on main and will be released shortly.

[PaulDance]

I saw febf3a1, and from what I understand, instead of calling set_soundness, one should now call refresh_tz, potentially as the first thing done in main, right? Also, I see that macOS was still not added to OS_HAS_THREAD_SAFE_ENVIRONMENT while it still could: was that intentional?

[jhpratt]

It was an oversight as I forgot to look back for verification. I was more focusing on the fact that the call will generally succeed out of the gate. I don't have a Mac to know if refresh_tz is needed at the start of the program, but on Linux (Fedora, specifically) I know that it is not.

cc @BlackHoleFox, who raised #610 originally. With the minimum supported macOS increased last year, I think this is sufficient for soundness now.

[jhpratt]

So right now, I think not having it is correct because the previously mentioned change didn't occur until Rust 1.74. As the MSRV is 1.67.1, that's too soon. I have recently considered bumping the MSRV to something a fair bit higher (≥1.74) in the coming months, which would unblock this. Adding it to my tracker for that reason.