src: make process.env.TZ setter clear tz cache

[bnoordhuis]

Since the presence of the libc and V8 timezone caches seem to be
a perennial source of confusion to users ("why doesn't it work?!"),
let's try to support that pattern by intercepting assignments to
the TZ environment variable and reset the caches as a side effect.

Fixes: #19974
CI: https://ci.nodejs.org/job/node-test-pull-request/14278/ - I expect the results will be... interesting.
CI: https://ci.nodejs.org/job/node-test-pull-request/14280/ - with tweaks
CI: https://ci.nodejs.org/job/node-test-pull-request/14287/ - one more tweak
CI: https://ci.nodejs.org/job/node-test-pull-request/23223/
CI: https://ci.nodejs.org/job/node-test-pull-request/23228/
CI: https://ci.nodejs.org/job/node-test-pull-request/23249/

[jeroenvollenbrock]

FWIW, i know chromium handles this by using OS specific code to hook into the OS’es notification system, and fire the V8 notification upon receiving a OS notification. I’m not sure if that approach completely obsoletes this one or if this one obsoletes that one, but i do think either both should be inside or outside the Node.JS feature scope to really avoid confusion. I’d be happy to make an additional PR if its deemed inside though :) Without this, issues like #4022 will probably continue to persist, while issues like #3449 will get resolved.

[bnoordhuis]

If anyone has a clue on what the timezone names should be on Windows... MSDN is lacking on that front.

@jeroenvollenbrock Chromium (on Linux) uses DBus. That's not a dependency we'd take on.

[jeroenvollenbrock]

I understand, however i don't think a dbus dependency will be necessary for this. Even though it is the easiest interface to implement for these notifications, it may be not even be present on the target platform, and multiple alternatives exist. Changing the timezone through systemd involves the following steps:

• change /etc/localtime symlink or copy file (could be watched, used by chromium, doesnt work in chrooted environments)
• notify kernel using settimeofday (can be manually polled using gettimeofday when RTC is using localtime)
  • Update vdso pages with new tz data (might be usable, needs investigation)
  • wake all armed and polled/read timerfds with TFD_TIMER_CANCEL_ON_SET set and deliver -ECANCEL (fd can be polled)
• notify subscribed dbus clients (can be subscribed to)

I know there has been a linux syscall in the works as well (time_change_notify), but i don't think it has been mainlined, and this one has also already been discussed as possible libuv notification (libuv/libuv#1176). The timerfds however, seem like an interesting possibility that uses very little resources. Just open a timerfd that is armed for a distant-future moment, poll for cancellations using libuv/poll, optionally check if /etc/localtime has changed, and reset the timezone caches.

• Windows has WM_TIMECHANGE
• MacOS has NSSystemTimeZoneDidChangeNotification

As for the windows timezone names. I don't think there is a complete list, but there have also been changes in it's behaviour across different versions and updates. (you may want to check out v8/#6031 as well)

[targos]

There's some documentation for Windows here: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset

[jeroenvollenbrock]

wake all armed and polled/read timerfds with TFD_TIMER_CANCEL_ON_SET set and deliver -ECANCEL (fd can be polled)

After taking a second look, i'm not sure if this also occurs when the system time itself doesnt change along with the timezone, probably needs more research as i'd guess a cancel is not required. (CLOCK_REALTIME is usually in UTC)

[bnoordhuis]

There's some documentation for Windows here: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/tzset

I've seen that but it doesn't name time zones except for one example that looks suspect (says GST is German Standard Time while the rest of the internet says it's Gulf or Georgia Standard Time.)

@jeroenvollenbrock You should open a new issue. To quickly address your points:

• TFD_TIMER_CANCEL_ON_SET is too new for Node.js to use. That was added around Linux 3.0 but our baseline is 2.6.18.
• We don't implement features unless there's a reasonable way to support it on all platforms. "Reasonable" implies "not a monumental effort."

[BridgeAR]

New CI before landing: https://ci.nodejs.org/job/node-test-pull-request/14434/

[BridgeAR]

I believe this is semver-major.

[BridgeAR]

CITGM https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1382/
CITGM (master) https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1383/

[BridgeAR]

Landed in 1d1ab76 🎉

[jasnell]

Marking this dont-land-on-v10x because it's too late to pull it in to the release.

[BridgeAR]

I missed CI failures before landing this. @bnoordhuis the Alpine builds fail with this.

[cjihrig]

Still LGTM. It would be nice to figure out what's up with the node-test-commit-custom-suites-freestyle issue though.

[addaleax]

@bnoordhuis That build job is python tools/test.py --worker, i.e. runs each test inside a Worker. Setting environment variables in a Worker does not affect the global per-process environment (unless that’s explicitly specified), so I assume that that’s the issue there. I think it’s okay to just skip the test if (!common.isMainThread) like we do elsewhere.

[srl295]

Still LGTM also. Presumably v8 manages ICU's default TZ if needed. (May not matter)

[bnoordhuis]

@addaleax Ah, that explains everything, thanks. :-)

I included your suggestion. New CI for good measure: https://ci.nodejs.org/job/node-test-pull-request/23249/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/23249/

[bnoordhuis]

It had to come a long way but this is merged in eba348b.