Skip to content

Ensure hard reference for frozen strings#1348

Merged
headius merged 1 commit intoruby:masterfrom
headius:avoid_frozen_literal_gc
Mar 5, 2026
Merged

Ensure hard reference for frozen strings#1348
headius merged 1 commit intoruby:masterfrom
headius:avoid_frozen_literal_gc

Conversation

@headius
Copy link
Contributor

@headius headius commented Feb 27, 2026

These frozen strings must be hard referenced until the identity has been tested, or an intervening GC could cause them to get collected and improperly fail the test. The cross-require global now points at the string in question and is cleared after the identity test.

@headius
Copy link
Contributor Author

headius commented Feb 27, 2026

Spurious failure in JRuby tests here was most likely due to unlucky GC timing. This should prevent such failures in the future:

https://github.com/jruby/jruby/actions/runs/22466746658/job/65074368459

@headius
Copy link
Contributor Author

headius commented Feb 27, 2026

Now RuboCop doesn't like the object_id comparison, but that was the original expectation. Changing to equal? would be an equivalent test.

@herwinw
Copy link
Member

herwinw commented Mar 3, 2026

Now RuboCop doesn't like the object_id comparison, but that was the original expectation. Changing to equal? would be an equivalent test.

For what it's worth: I think Rubocop has some value to keep the formatting/indentation consistent, but a lot of code in this repository is not idiomatic Ruby on purpose.
For Natalie, we've switched to syntax_tree, which is just formatting and nothing else. I'm honestly thinking this might be a better fit for ruby-spec too.

@eregon
Copy link
Member

eregon commented Mar 5, 2026

Looks good.
Let's switch to equal?, I think in general it's not good to compare object_ids anyway, and also requesting the object_id might have a side effect like adding the object in some weakmap on CRuby or so.

@headius
Copy link
Contributor Author

headius commented Mar 5, 2026

Looks good. Let's switch to equal?, I think in general it's not good to compare object_ids anyway, and also requesting the object_id might have a side effect like adding the object in some weakmap on CRuby or so.

Will do and then merge.

@headius headius force-pushed the avoid_frozen_literal_gc branch from be665d0 to 9785abf Compare March 5, 2026 16:37
These frozen strings must be hard referenced until the identity
has been tested, or an intervening GC could cause them to get
collected and improperly fail the test. The cross-require global
now points at the string in question and is cleared after the
identity test.
@headius headius force-pushed the avoid_frozen_literal_gc branch from 9785abf to a214323 Compare March 5, 2026 16:43
@headius
Copy link
Contributor Author

headius commented Mar 5, 2026

GHA is down again so I'm merging without the final run. Tested locally on 3.4 and 4.0, it should be fine.

@headius headius merged commit 2e3e131 into ruby:master Mar 5, 2026
@headius headius deleted the avoid_frozen_literal_gc branch March 5, 2026 17:03
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

Successfully merging this pull request may close these issues.

3 participants