Skip to content

Share array entities if possible with ary.replace(frozen_ary) #6576

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.

Sign up for GitHub

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

Open
dearblue wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Share array entities if possible with ary.replace(frozen_ary) #6576

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ ary_replace(mrb_state *mrb, struct RArray *a, struct RArray *b)
mrb_write_barrier(mrb, (struct RBasic*)a);
return;
}
if (!mrb_frozen_p(b) && len > ARY_REPLACE_SHARED_MIN) {
if (len > ARY_REPLACE_SHARED_MIN) {
Copy link

@gemini-code-assist gemini-code-assist bot Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This change removes the !mrb_frozen_p(b) check, which allows ary_make_shared(mrb, b) to be called on a frozen array b.

The ary_make_shared() function modifies the internal state of the array passed to it. Specifically, it updates the aux union member to point to a mrb_shared_array struct and sets the MRB_ARY_SHARED flag:

// in ary_make_shared(mrb, a) where a is b
a->as.heap.aux.shared = shared;
ARY_SET_SHARED_FLAG(a);

Modifying a frozen object, even its internal metadata, violates the immutability guarantee of frozen objects in Ruby. This could lead to unexpected behavior and subtle bugs if other parts of the system rely on frozen objects being truly immutable.

While mrb_obj_freeze() for arrays does shrink the capacity to match the length (avoiding a realloc inside ary_make_shared()), the modification of the RArray struct itself remains a serious concern.

Is this change in the semantics of frozen arrays intentional? If so, this is a significant change that should be carefully documented. Otherwise, this change appears to introduce a critical bug.

An alternative to achieve sharing from a frozen array would require a mechanism that does not modify the source frozen array, which would likely be a more significant change involving array and GC management.

Copy link
Contributor Author

@dearblue dearblue Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change in the semantics of frozen arrays intentional? If so, this is a significant change that should be carefully documented. Otherwise, this change appears to introduce a critical bug.

If this is to be a future concern, I think an equivalent to RSTR_NOFREE_P() needs to be introduced at this time.

ary_make_shared(mrb, b);
goto shared_b;
}
Expand Down
Loading