10000 Fix too early writebarrier in tally_up by jhawthorn · Pull Request #13629 · ruby/ruby · GitHub
[go: up one dir, main page]
Skip to content

Fix too early writebarrier in tally_up #13629

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

Jump to bottom
Merged
merged 1 commit into from
Jun 17, 2025
Merged

Fix too early writebarrier in tally_up #13629

merged 1 commit into from
Jun 17, 2025
+3 −2

Conversation

jhawthorn
Copy link
Member
@jhawthorn jhawthorn commented Jun 16, 2025 •
edited
Loading

This is a "too early" write barrier which I found this using my wbcheck tool (See #13557). I added an option to re-verify objects on the first possible GC after they have a write barrier.

So what could have happened is:

  1. The previous RB_OBJ_WRITTEN is hit in tally_up, but there is not yet GC happening so nothing is recorded
  2. st_update realizes it needs to resize the table, and so allocates. This triggers GC which incrementally marks hash, but group has not yet been written
  3. By the time the rest of GC happens the other parents of group are never marked and freed, and group is never marked
$ RUBY_GC_LIBRARY=wbcheck WBCHECK_VERIFY_AFTER_WB=1 ./miniruby -e '(0...100).map(&:to_s).tally'

WBCHECK ERROR: Missed write barrier detected!
  Parent object: 0x5fe8145822d0 (wb_protected: true)
    rb_obj_info_dump: 0x00005fe8145822d0 T_HASH/Hash [S] 32
  Reference counts - snapshot: 17, writebarrier: 16, current: 33, missed: 1
  Missing reference to: 0x5fe814585940
    rb_obj_info_dump: 0x00005fe814585940 T_STRING/String  len: 2, capa: 15 "16"

@jhawthorn
Fix too early writebarrier in tally_up
18cdd23 
After returning from the callback in st_update is the point that the
hash table may be resized, which could trigger a GC and mark the table
being used for the tally.

    RUBY_GC_LIBRARY=wbcheck WBCHECK_VERIFY_AFTER_WB=1 ./miniruby -e '(0...100).map(&:to_s).tally'
@jhawthorn jhawthorn mentioned this pull request Jun 17, 2025
Merged
@launchable-app Launchable app
Copy link

❌ Tests Failed

✖️no tests failed ✔️62001 tests passed(2 flakes)

@jhawthorn jhawthorn merged commit a7dc515 into ruby:master Jun 17, 2025
96 of 100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nobu nobu nobu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jhawthorn @nobu
0