Skip to content
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

Backport cvar clone bug fix for 19379 to 3.2 #7888

Merged

Conversation

eileencodes
Copy link
Contributor

@eileencodes eileencodes commented Jun 1, 2023

This PR backports #7275 and #7265 to ruby_3_2. I know this is normally done by the release manager but since there were conflicts I figured it would be helpful to do the backport and open a PR.

Fixes: https://bugs.ruby-lang.org/issues/19394

@matzbot matzbot requested a review from a team June 1, 2023 16:21
@eileencodes eileencodes force-pushed the backport-cvar-clone-bug-fix-19379 branch from 6df5a25 to 81863a6 Compare June 1, 2023 16:25
@eileencodes eileencodes changed the title Backport cvar clone bug fix 19379 Backport cvar clone bug fix for 19379 to 3.2 Jun 1, 2023
@eileencodes
Copy link
Contributor Author

Blocked by #7902

eileencodes and others added 3 commits June 5, 2023 14:49
When a class with a class variable is cloned we need to also copy the
cvar cache table from the original table to the clone. I found this bug
while working on fixing [Bug #19379]. While this does not fix that bug
directly it is still a required change to fix another bug revealed by
the fix in ruby#7265

This needs to be backported to 3.2.x and 3.1.x.

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
The class variable cache that was added in
ruby#4544 changed the behavior of class
variables on cloned classes. As reported when a class is cloned AND a
class variable was set, and the class variable was read from the
original class, reading a class variable from the cloned class would
return the value from the original class.

This was happening because the IC (inline cache) is stored on the ISEQ
which is shared between the original and cloned class, therefore they
share the cache too.

To fix this we are now storing the `cref` in the cache so that we can
check if it's equal to the current `cref`. If it's different we don't
want to read from the cache. If it's the same we do. Cloned classes
don't share the same cref with their original class.

This will need to be backported to 3.1 in addition to 3.2 since the bug
exists in both versions.

We also added a marking function which was missing.

Fixes [Bug #19379]

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
We were missing the write barrier for class_value to cref. This should
fix the segv we were seeing in http://ci.rvm.jp/logfiles/brlog.trunk-gc-asserts.20230601-165052

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
@eileencodes eileencodes force-pushed the backport-cvar-clone-bug-fix-19379 branch from 81863a6 to 35323e9 Compare June 5, 2023 18:49
@hsbt hsbt added the Backport label Jun 29, 2023
@nagachika nagachika self-requested a review June 29, 2023 10:12
@nagachika nagachika merged commit 038913f into ruby:ruby_3_2 Jul 1, 2023
87 of 89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants