-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Milestone
Comments
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
added a commit
to iconara/jruby
that referenced
this issue
Jun 8, 2017
Thanks for the report and PR! |
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
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:The same goes for
#localtime
: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:It looks like problem lies in these two lines:
jruby/core/src/main/java/org/jruby/RubyTime.java
Line 512 in d7c5b10
jruby/core/src/main/java/org/jruby/RubyTime.java
Line 528 in d7c5b10
I've looked through the rest of
RubyTime.java
and it looks like these are the only place wheredt
ornsec
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?The text was updated successfully, but these errors were encountered: