Bug #17131: Time.at(time) != time in certain cases

• Status: Closed
• Assignee: -
• Target version: -
• ruby -v: 2.0.0 - 2.7.0
• Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN

[phil_pirozhkov (Phil Pirozhkov)]

Problem

According to the spec:

    describe "with an argument that responds to #to_r" do
      it "coerces using #to_r" do
        o = mock_numeric('rational')
        o.should_receive(:to_r).and_return(Rational(5, 2))
        Time.at(o).should == Time.at(Rational(5, 2))
      end
    end

Time.at applies to_r to its argument if it responds to to_r. It doesn't specify though what the result should be.

It preserves the value with Time class:

time = Time.now
time.nsec # => 110716000
time.nsec == Time.at(time).nsec # => true

time = Time.at(946684800, 123456789, :nsec)
time.nsec # => 123456789
Time.at(time).nsec # => 123456789

and stdlib's DateTime class:

require 'time'
dt = DateTime.now
dt.to_time.nsec # => 111439000
dt.to_time.nsec == Time.at(dt.to_time).nsec # => true

However, ActiveSupport::TimeWithZone, which mimics Time but doesn't inherit from Time and rather delegates to it, does not preserve the value:

t=Time.current
t.nsec # => 3882000
Time.at(t).nsec # => 3881931

This may look like rounding issue:

The lowest digits of to_f and nsec are different because IEEE 754 double is not accurate enough to represent the exact number of nanoseconds since the Epoch.

But that doesn't explain why Time doesn't suffer from this rounding issue.

Why?

The ecosystems have provided workarounds:

• https://github.com/travisjeffery/timecop/pull/70
• https://github.com/rails/rails/pull/9403

But they do not work because to_r is not called. And it spirals out to downright crutches

• https://github.com/rails/rails/pull/35713

and duct tape

• https://github.com/rspec/rspec-rails/pull/2304

This results in end user specs that do not handle nano-, micro-, or even milliseconds precision:

it 'is scheduled in 5 seconds' do
  expect { SayHiJob.perform_in_five }
    .to have_enqueued_job(SayHiJob).at(a_value_within(1.second).of(Time.current))
end

A related Rails issue is

• https://github.com/rails/rails/issues/38831

Investigation

With a simpler example of a class that pretends to be coercible to Time:

RationalTime = Class.new do
  def respond_to?(method_name, *)
    puts method_name
    super
  end
  def to_r
    @r
  end
  def initialize(r)
    @r = r
  end
end

r = Rational(858710000, 3)
rt = RationalTime.new(r)
puts Time.at(rt)

the result is rather frustrating:

to_r # <= `Time.at` checked if the class responds to `.to_r`
to_int # but also checked if it responds to `to_int`!
1.rb:19:in `at': can't convert RationalTime into an exact number (TypeError)
        from 1.rb:19:in `<main>'

It seems to fail at the following test:

    describe "with an argument that responds to #to_r" do
      it "coerces using #to_r" do

We see that respond_to? was called for to_int. Let's implement it:

RationalTime = Class.new do
  def to_r
    Rational(1111, 3)
  end

  def to_int
    100
  end
end

puts Time.at(RationalTime.new).nsec # => 333333333
puts Time.at(RationalTime.new).to_f # => 370.3333333333333

The returned value is far away from 100, but it raises no errors, and our rational part is finally used.

Nanoseconds rounding

Time.now returns time with no nanosecond precision, e.g.:

Time.now.nsec # => 225_852_000

I do not know whether defining is_a? and kind_of? makes any difference:

RationalTime = Class.new do
  def is_a?(klass)
    klass == ::Time || super
  end

  def kind_of?(klass)
    klass == ::Time || super
  end

  def to_r
    Rational(1111, 3)
  end

  def to_int
    100
  end
end

Digging deeper

The following mostly guessing.

According to Ruby core spec that covers Time.at with a Rational:

    it "roundtrips a Rational produced by #to_r" do
      t = Time.now()
      t2 = Time.at(t.to_r)

      t2.should == t
      t2.usec.should == t.usec
      t2.nsec.should == t.nsec
    end

it seems that Time.at copies the underlying time data without modification if the argument is detected to be Time (Ruby source:

else if (IsTimeval(time)) {
  struct time_object *tobj, *tobj2;
  GetTimeval(time, tobj);
  t = time_new_timew(klass, tobj->timew);
  GetTimeval(t, tobj2);
  TZMODE_COPY(tobj2, tobj);
}

where IsTimeval is defined by:

#define IsTimeval(obj) rb_typeddata_is_kind_of((obj), &time_data_type)

where rb_typeddata_is_kind_of is defined as (error.c?):

rb_typeddata_is_kind_of(VALUE obj, const rb_data_type_t *data_type)
{
    if (!RB_TYPE_P(obj, T_DATA) ||
        !RTYPEDDATA_P(obj) || !rb_typeddata_inherited_p(RTYPEDDATA_TYPE(obj), data_type)) {
        return 0;
    }
    return 1;
}

The hope that IsTimeval evaluates to true for an impersonating class is debunked by an experimental result:

RTInteger = Class.new do
  def is_a?(klass)
    klass == ::Time || super
  end

  def kind_of?(klass)
    klass == ::Time || super
  end

  def to_int
    100
  end
end

Time.at(RTInteger.new) # => 1970-01-01 02:01:40 +0200

Our time passes through a series of lossy transformations (1, 2):

        timew = rb_time_magnify(v2w(num_exact(time)));
        t = time_new_timew(klass, timew);

v2w(VALUE v)
{
    if (RB_TYPE_P(v, T_RATIONAL)) {
        if (RRATIONAL(v)->den != LONG2FIX(1))
            return WIDEVAL_WRAP(v);
        v = RRATIONAL(v)->num;
    }

There is a check for it to be a Rational, but I can't find where respond_to?(:to_r) call comes from. In any case, it ends up deciding not to call .to_r for some reason.

Regression or undefined behavior?

Let's run the snippet from the beginning of this ticket with RationalTime on different Ruby versions/implementations.

RationalTime = Class.new do
  def to_r
    Rational(1111, 3)
  end

  def to_int
    100
  end
end

puts Time.at(RationalTime.new).nsec
puts Time.at(RationalTime.new).to_f

With to_int defined:

Time.at(RationalTime.new).nsec # => 333333333
Time.at(RationalTime.new).to_f # => 370.3333333333333

Without to_int defined:

TypeError: can't convert RationalTime into an exact number
        from (irb):7:in `at'
        from (irb):7
        from /Users/pirj/.rvm/rubies/ruby-2.0.0-p648/bin/irb:12:in `<main>'

It's consistent from 2.0.0 throughout 2.7.0.

JRuby (jruby 9.3.0.0-SNAPSHOT (2.6.5) 2020-08-25 2f0c49000a OpenJDK 64-Bit Server VM 14.0.1+14 on 14.0.1+14 +jit [darwin-x86_64]):
with to_int:

Time.at(RationalTime.new).nsec # => 333333333
Time.at(RationalTime.new).to_f # => 370.33333333300004

Without to_int:

Traceback (most recent call last):
       11: from /Users/pirj/.rvm/rubies/jruby-head/bin/jruby_executable_hooks:24:in `<main>'
       10: from org/jruby/RubyKernel.java:1117:in `eval'
        9: from /Users/pirj/.rvm/rubies/jruby-head/bin/irb:23:in `<main>'
        8: from org/jruby/RubyKernel.java:1078:in `load'
        7: from /Users/pirj/.rvm/rubies/jruby-head/lib/ruby/gems/shared/gems/irb-1.0.0/exe/irb:11:in `<main>'
        6: from org/jruby/RubyKernel.java:1263:in `catch'
        5: from org/jruby/RubyKernel.java:1263:in `catch'
        4: from org/jruby/RubyKernel.java:1524:in `loop'
        3: from org/jruby/RubyKernel.java:1117:in `eval'
        2: from (irb):7:in `evaluate'
        1: from org/jruby/RubyTime.java:1329:in `at'
TypeError (can't convert RationalTime into an exact number)

Additional note: precision

The spec:

    describe "with an argument that responds to #to_r" do
      it "coerces using #to_r" do
        o = mock_numeric('rational')
        o.should_receive(:to_r).and_return(Rational(5, 2))
        Time.at(o).should == Time.at(Rational(5, 2))
      end
    end

doesn't provide much precision, as Rational(5, 2) is 2.5. Obviously, microseconds and nanoseconds are all zero, and this time it has 500 milliseconds. Rational(22, 7) would yield more floating digits to make assertions on nanosecond precision.

How to fix

The point is to let Time.at adhere to duck typing. If the only argument to Time.at responds to to_r, it should be used, and to_int shouldn't be required, since it's not used down the lines anyway.

In addition to that, it would be nice to adjust the specs so that if a Rational or an object that responds to to_r is passed, then all of its precision is kept, i.e:

    describe "with an argument that responds to #to_r" do
      it "coerces using #to_r" do
        o = mock_numeric('rational')
        o.should_receive(:to_r).and_return(Rational(22, 7))
        Time.at(o).should == Time.at(Rational(22, 7))
        Time.at(o).nsec.should == 142857142
        Time.at(o).to_f.round(8).should == 3.14285714
        Time.at(o).to_f.round(9).should == 3.142857143 # Not sure if JRuby is capable of this precision though
      end
    end

[jeremyevans0 (Jeremy Evans)]

The check for to_int in addition to to_r is deliberate. The related code comment states: test to_int method availability to reject non-Numeric objects such as String, Time, etc which have to_r method. So that is not a bug.

I've read the entire post and other than that issue (which isn't a bug), I'm not sure where you think there is an issue. Assuming that the value passed to Time#at responds to to_r and returns a rational and responds to to_int, the precision specified by the rational should be kept. There external issues you link to all stem from use of ActiveSupport, not from Ruby itself.

[sawa (Tsuyoshi Sawada)]

• Description updated (diff)

[jeremyevans0 (Jeremy Evans)]

• Status changed from Open to Closed

@nobu (Nobuyoshi Nakada) committed a spec that shows that defining to_r without to_int is expected to raise an error (7e1fddba4a609cb7bf4a696eccd892e68753bb21). I think that is sufficient to handle this issue. If you would like additional documentation added, please reply. Currently, the documentation states a single argument to Time.at, if it isn't a time, should be an Integer, Float, Rational, or other Numeric.

[phil_pirozhkov (Phil Pirozhkov)]

jeremyevans0 (Jeremy Evans) wrote in #note-3:

a spec that shows that defining to_r without to_int is expected to raise an error
I think that is sufficient to handle this issue

All clear, thanks a lot. I'll handle the usages to adhere to this protocol.