Project

General

Profile

Actions

Bug #17563

closed

FrozenError raised from Module#const_set when receiver is not frozen

Added by ryannevell (Ryan Nevell) about 3 years ago. Updated about 3 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 3.1.0dev (2021-01-19T16:58:26Z master a8dc5156e1) [x86_64-darwin20]
[ruby-core:102172]

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) about 3 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) about 3 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) about 3 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) about 3 years ago

I’m afraid if it’s safe when multi-ractor.

Actions #5

Updated by nobu (Nobuyoshi Nakada) about 3 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)

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0