Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix os.date() for timezone change awareness #473

Closed
wants to merge 1 commit into from

Conversation

BuBuaBu
Copy link

@BuBuaBu BuBuaBu commented Jan 30, 2019

On POSIX target, system timezone change are not taken into account.
To reproduce,

  1. call os.date()
  2. change your timezone
  3. call os.date() within the same luajit instance

On POSIX target, os.date use localtime_r to retrieve time.
On other target, the function localtime is used. But there is a behaviour
diference between these two function. localtime acts as if it called tzset
which localtime_r don't.

To fix the issue tzset is called before localtime_r.

On POSIX target, system timezone change are not taken into account.
To reproduce,
1. call os.date()
2. change your timezone
3. call os.date() within the same luajit instance

On POSIX target, os.date use localtime_r to retrieve time.
On other target, the function localtime is used. But there is a behaviour
diference between these two function. localtime acts as if it called tzset
which localtime_r don't.

To fix the issue tzset is called before localtime_r.
@siddhesh
Copy link

I've included this patch into my repo[1] and in Fedora rawhide, thanks.

[1] https://github.com/siddhesh/LuaJIT/tree/v2.1

@MikePall
Copy link
Member

MikePall commented Dec 8, 2019

This is counter-productive as it kills performance, especially for server workloads. With this change os.date("") is 14 times slower (tested on Linux/glibc).

Timezone changes at runtime are only an issue for mobile OS (note: we are not talking about daylight saving times changes, as these are part of the timezone definition here). Android and iOS already handle this by doing an optimized check for timezone changes, even in the _r variants of the time functions. Also, they send a broadcast to all apps on timezone changes, and apps better listen to this and deal with it.

@MikePall MikePall closed this Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants