Bug #17563
closedFrozenError raised from Module#const_set when receiver is not frozen
Description
The following code executed without error on Ruby 2.7.1 and many earlier 2.* versions. The behavior has changed on Ruby 3.0.0 and now raises a Frozen Error:
% ruby -e 'Module.new.const_set(:Foo, Class.new.freeze)'
-e:1:in `const_set': can't modify frozen #<Class:#<Class:0x00007fe883886698>>: #<Class:0x00007fe883886698> (FrozenError)
from -e:1:in `<main>'
It seems that const_set is attempting to modify the (frozen) object being passed in, not just the receiver of the const_set call. This seems to only happen if that object is a Module or Class.
This may be due to this change to rb_const_set.
On 2.7.1, no action was taken if parental_path
was nil
:
int parental_path_permanent;
VALUE parental_path = classname(klass, &parental_path_permanent);
if (!NIL_P(parental_path)) {
if (parental_path_permanent && !val_path_permanent) {
set_namespace_path(val, build_const_path(parental_path, id));
}
else if (!parental_path_permanent && NIL_P(val_path)) {
rb_ivar_set(val, tmp_classpath, build_const_path(parental_path, id));
}
}
On 3.0, a temporary class path is created, and then it goes on to call rb_ivar_set
on val
, which will error out if val.frozen? == true
:
int parental_path_permanent;
VALUE parental_path = classname(klass, &parental_path_permanent);
if (NIL_P(parental_path)) {
int throwaway;
parental_path = rb_tmp_class_path(klass, &throwaway, make_temporary_path);
}
if (parental_path_permanent && !val_path_permanent) {
set_namespace_path(val, build_const_path(parental_path, id));
}
else if (!parental_path_permanent && NIL_P(val_path)) {
rb_ivar_set(val, tmp_classpath, build_const_path(parental_path, id));
}
Updated by ryannevell (Ryan Nevell) almost 4 years ago
I've tried simply replacing rb_ivar_set
with ivar_set
, which skips the freeze check, but otherwise behaves the same. Since the instance variable "__tmp_classpath__"
really looks like an internal property, modifying the frozen object to add this doesn't seem like it could do any harm.
Updated by marcandre (Marc-Andre Lafortune) almost 4 years ago
- Assignee set to jeremyevans0 (Jeremy Evans)
Good bug hunting 👍
Your solution is sounds like the right one, I imagine @jeremyevans0 (Jeremy Evans) will confirm.
Updated by jeremyevans0 (Jeremy Evans) almost 4 years ago
- Assignee changed from jeremyevans0 (Jeremy Evans) to nobu (Nobuyoshi Nakada)
Using ivar_set
seems reasonable to me, but I'd like to get @nobu's input as to whether this is acceptable.
Updated by nobu (Nobuyoshi Nakada) almost 4 years ago
I’m afraid if it’s safe when multi-ractor.
Updated by nobu (Nobuyoshi Nakada) almost 4 years ago
- Status changed from Open to Closed
Applied in changeset git|565aeb81e0886c835888a425e5d05ed99fb03238.
Skip freezing check on setting temporary class path [Bug #17563]
Co-authored-by: ryannevell (Ryan Nevell) ryan.nevell@gmail.com