Skip to content

Fix generic_ivar_set_shape_field for table rebuild #13589

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

Merged
merged 2 commits into from
Jun 14, 2025

Conversation

jhawthorn
Copy link
Member

@jhawthorn jhawthorn commented Jun 11, 2025

Previously GC could trigger a table rebuild of the generic fields st_table in the middle of calling the st_update callback. This could cause entries to be reallocated or rearranged and the update to be for the wrong entry.

This commit adds an assertion to make that case easier to detect, and replaces the st_update with a separate st_lookup and st_insert.

This should be an additional fix to the issue we've been seeing in test_array.rb in CI (which has also had another workaround).

We used this as a (somewhat) reliable reproduction:

objs = 10_000.times.map do
  a = []
  a.instance_variable_set(:@a, 1)
  a
end

GC.stress = true
GC.auto_compact = true

steps = 1000.times.map do
  a = []
  a.instance_variable_set(:@a, 1)
  a.instance_variable_set(:@b, 1)
  a.instance_variable_set(:@c, 1)
  a.instance_variable_set(:@d, 1)
  a.instance_variable_set(:@e, 1)
  a
end

GC.stress = false
GC.auto_compact = false

@byroot
Copy link
Member

byroot commented Jun 11, 2025

We probably should check older versions, I fear this bug is present on at least 3.4, possibly older.

However to trigger on older versions you need 9 ivars (or 10 in debug mode).

@casperisfine
Copy link
Contributor

I was able to repro on 3.4, and the code had a similar shape for way over a decade, so I opened a report so we can backport: https://bugs.ruby-lang.org/issues/21438

@byroot
Copy link
Member

byroot commented Jun 13, 2025

@jhawthorn shall we merge or is there some more changes you want to make?

jhawthorn and others added 2 commits June 13, 2025 18:54
[Bug #21438]

Previously GC could trigger a table rebuild of the generic fields
st_table in the middle of calling the st_update callback. This could
cause entries to be reallocated or rearranged and the update to be for
the wrong entry.

This commit adds an assertion to make that case easier to detect, and
replaces the st_update with a separate st_lookup and st_insert.

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
Copy link

Tests Failed

✖️no tests failed ✔️62022 tests passed(1 flake)

@jhawthorn jhawthorn enabled auto-merge (rebase) June 14, 2025 06:24
@jhawthorn jhawthorn merged commit 39697ff into ruby:master Jun 14, 2025
87 of 89 checks passed
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