Preserve DST boolean to fix marshalling Time in JRuby #11911

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

Store isdst as an instance variable to eliminate the one hour time
difference when doing Marshal.load(Marshal.dump(Time.now)) between
active_support/core_ext/time/marshal and JRuby Marshal.

@mikeadmire mikeadmire Preserve DST boolean to fix marshalling Time in JRuby
Store isdst as an instance variable to eliminate the one hour time
difference when doing Marshal.load(Marshal.dump(Time.now)) between
active_support/core_ext/time/marshal and JRuby Marshal.
d879d52
Owner

pixeltrix commented Aug 16, 2013

I'm not sure that whether the date is DST or not is the culprit, e.g:

>> ENV['TZ'] = 'US/Eastern'
=> "US/Eastern"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 03:02:52 -0400
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 04:03:10 -0400

This replicates your example, however if I use a different local timezone:

>> ENV['TZ'] = 'Europe/London'
=> "Europe/London"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 08:06:01 +0100
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 08:06:31 +0600

In this example the time remains the same but the offset is shifted by 5 hours. I'll investigate further to see what the real cause of the problem is.

pixeltrix was assigned Aug 16, 2013

@pixeltrix you're right. I'm seeing the same thing. US Pacific, Mountain, and Central time zones are consistent with US/Eastern time.

>> ENV['TZ'] = 'America/Los_Angeles'
=> "America/Los_Angeles"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 05:53:34 -0700
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 06:53:43 -0700

Hawaii time remains the same and there is no time zone shift (no DST).

>> ENV['TZ'] = 'Pacific/Honolulu'
=> "Pacific/Honolulu"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 02:54:28 -1000
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 02:54:42 -1000

Melbourne both time and offset shift.

>> ENV['TZ'] = 'Australia/Melbourne'
=> "Australia/Melbourne"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 22:49:00 +1000
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-16 23:49:11 -0400

There is something else going on here. I'll keep digging too.

This patch only seems to perform as expected when the environment is set to US time zones. It also appears that the above behavior changes from jruby 1.7.4 to 1.7.5.dev.

>> JRUBY_VERSION
=> "1.7.5.dev"
>> ENV['TZ'] = 'Australia/Melbourne'
=> "Australia/Melbourne"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-23 23:55:16 +1000
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-24 08:55:32 +0400
>> JRUBY_VERSION
=> "1.7.5.dev"
>> ENV['TZ'] = 'Europe/London'
=> "Europe/London"
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-23 15:00:03 +0100
>> require 'active_support/time'
=> true
>> Marshal.load(Marshal.dump(Time.now))
=> 2013-08-23 03:00:10 -0600

It still shifts the offset and also seems to negate the offset from 1.7.4.

Actually, this patch relies on Time.local(*arr) which works very suspicious in jruby (all versions up to 1.7.4). Look at this:

jruby-1.7.4 :016 > JRUBY_VERSION
 => "1.7.4" 
jruby-1.7.4 :017 > ENV['TZ'] = 'Europe/London'
 => "Europe/London" 
jruby-1.7.4 :018 > t1 = Time.now
 => 2013-09-14 04:12:12 +0100 
jruby-1.7.4 :019 > t1.zone
 => "BST" 
jruby-1.7.4 :020 > t1.to_a
 => [12, 12, 4, 14, 9, 2013, 6, 257, true, "BST"] 
jruby-1.7.4 :021 > t2 = Time.local(*(t1.to_a))
 => 2013-09-14 03:12:12 +0600 
jruby-1.7.4 :022 > t2.to_a
 => [12, 12, 3, 14, 9, 2013, 6, 257, false, "BDT"] 
jruby-1.7.4 :023 > t1 == t2
 => false

I must admit that for EST it works correctly:

jruby-1.7.4 :025 >   JRUBY_VERSION
 => "1.7.4" 
jruby-1.7.4 :026 > ENV['TZ'] = 'US/Eastern'
 => "US/Eastern" 
jruby-1.7.4 :027 > t1 = Time.now
 => 2013-09-13 23:13:48 -0400 
jruby-1.7.4 :028 > t1.zone
 => "EDT" 
jruby-1.7.4 :029 > t1.to_a
 => [48, 13, 23, 13, 9, 2013, 5, 256, true, "EDT"] 
jruby-1.7.4 :030 > t2 = Time.local(*(t1.to_a))
 => 2013-09-13 23:13:48 -0400 
jruby-1.7.4 :031 > t2.to_a
 => [48, 13, 23, 13, 9, 2013, 5, 256, true, "EDT"]

I am not that savvy with time zones so probably this behavior is programmed with a purpose. I'm personally interested to figure it out and plan to spend some time digging this. Maybe we should just create a patch for Time.local for jruby implementation...

Look at this screenshot from wikipedia: http://bit.ly/1azDwdK. It turns out same abbreviations can be used for different time zones. As an example, BST is used for both British (+01) and Bangladesh (+06) Standard Times. Here's what happens in jruby for Europe/London: we dump London Time as British and load it as Bangladesh. That's why there's a difference:

jruby-1.7.4 :016 > JRUBY_VERSION
 => "1.7.4" 
jruby-1.7.4 :017 > ENV['TZ'] = 'Europe/London'
 => "Europe/London" 
jruby-1.7.4 :018 > t1 = Time.now
 => 2013-09-14 04:12:12 +0100 
jruby-1.7.4 :021 > t2 = Time.local(*(t1.to_a))
 => 2013-09-14 03:12:12 +0600 

It's a kind of logical issue and I am not sure so far how it should be fixed.

Just found that jruby falls back on sun.util.calendar.ZoneInfo.getAvailableIDs() to get a list of available time zones and here's what it returns:

import sun.util.calendar.ZoneInfo;
print(ZoneInfo.getAvailableIDs());

/*
...
Asia/Thimphu,
Asia/Yekaterinburg,
BST,
Etc/GMT-6,
Indian/Chagos,
Asia/Rangoon,
...
*/

As you can see BST has an offset GMT-6 (which means actually GTM+6 for ruby). It looks like there's no BST abbreviation for British Standard Time in java while BST for ruby means British Standard Time. The only workaround I can see now is to use GTM+-offset format for "non-standard" time zones while marshaling in jruby.

atambo referenced this pull request in jruby/jruby Sep 22, 2013

Merged

Date library using Joda-Time (v2) #890

Contributor

nateberkopec commented May 5, 2015

@andrewtabit What do you mean by "non-standard time zone"? I can reproduce this with an Eastern Daylight Time clock in JRuby 1.7.5:

irb(main):001:0> Marshal.load(Marshal.dump(Time.now))
=> 2015-05-05 09:26:16 -0400
irb(main):002:0> require 'active_support/core_ext/time/marshal'
=> true
irb(main):003:0> Marshal.load(Marshal.dump(Time.now))
=> 2015-05-05 17:26:27 +0400
irb(main):004:0> t1 = Time.now
=> 2015-05-05 09:28:25 -0400
irb(main):005:0> t1.zone
=> "EDT"
Member

robin850 commented Jul 22, 2015

It looks like this is no longer a problem on JRuby 1.7.21 and 9000 so we can certainly close this ; moreover this file is now deprecated. Thanks for your contribution anyway! :-)

robin850 closed this Jul 22, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment