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

Time#utc and #localtime can mutate a frozen object #4655

Closed
iconara opened this issue Jun 8, 2017 · 4 comments
Closed

Time#utc and #localtime can mutate a frozen object #4655

iconara opened this issue Jun 8, 2017 · 4 comments

Comments

@iconara
Copy link
Contributor

iconara commented Jun 8, 2017

Environment

This is one of the relevant environments, but the issue can be replicated in all environments I've tested:

$ ruby -v
jruby 9.1.8.0 (2.3.1) 2017-03-06 90fc7ab Java HotSpot(TM) 64-Bit Server VM 25.112-b16 on 1.8.0_112-b16 +jit [darwin-x86_64]
$ uname -a
Darwin quattro.local 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64

Expected Behavior

When I freeze a Time instance I expect to not be able to run #utc since that method changes the instance:

irb> Time.new.freeze.utc
RuntimeError: can't modify frozen Time
	from (irb):1:in `utc'
	from (irb):1

The same goes for #localtime:

irb>  Time.new.utc.freeze.localtime
RuntimeError: can't modify frozen Time
	from (irb):1:in `localtime'
	from (irb):1

Actual Behavior

In all versions of JRuby I have tested (1.7.19, 1.7.23, 1.7.26, 9.0.5.0, 9.1.8.0) I can modify frozen Time instances:

irb> t = Time.now
=> 2017-06-08 10:12:04 +0200
irb> t.freeze
=> 2017-06-08 10:12:04 +0200
irb> t.frozen?
=> true
irb> t.utc
=> 2017-06-08 08:12:04 UTC
irb> t
=> 2017-06-08 08:12:04 UTC
irb> t.localtime
=> 2017-06-08 10:12:04 +0200
irb> t
=> 2017-06-08 10:12:04 +0200

It looks like problem lies in these two lines:

I've looked through the rest of RubyTime.java and it looks like these are the only place where dt or nsec are changed except for at initialization time.

Even though this is inconsistent behaviour with MRI it could potentially break a lot of code so I assume it's not something that could be rolled out in a patch release.

It looks like an easy fix, and I'd love to submit a PR, but I need is a pointer to where to add a test for it. Would adding a test/test_time_freeze.rb be the right thing to do?

@headius
Copy link
Member

headius commented Jun 8, 2017

The best place to add a test would be our copy of https://github.com/ruby/spec, in spec/ruby under our repo. The fix sounds like it just needs a frozen check, because presumably MRI modifies the object in the same way?

@iconara
Copy link
Contributor Author

iconara commented Jun 8, 2017

@headius thank you for the pointer, I have opened a PR with a fix: #4657.

@headius headius added this to the JRuby 9.1.11.0 milestone Jun 9, 2017
@headius headius closed this as completed Jun 9, 2017
@headius
Copy link
Member

headius commented Jun 9, 2017

Thanks for the report and PR!

@iconara
Copy link
Contributor Author

iconara commented Jun 9, 2017

My pleasure, thanks for the help.

eregon pushed a commit to ruby/spec that referenced this issue Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants