[pull] master from ruby:master - #1356
Merged
Merged
Conversation
Keep complete dependency rules in upstream gems while storing only source mappings in the Ruby repository.
Keep generated rules even when manual rules cover them so source mappings can round-trip between default gems and ruby/ruby.
test_global_variables called nonexistent assert_not_include?, raising NoMethodError when the suite runs with RUBY_BOX=1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
coroutine_thread_terminated() leaves the Ractor's living set before handing the scheduler slot over, then keeps using th for the rest of the function: thread_sched_to_dead_common() reads th->nt and deregisters th from the barrier, and the epilogue then stashes th->nt and clears th->sched.context. Off the set th is unreachable, and once deregistered no barrier waits for it, so a sweep in another Ractor can free it mid-epilogue. The epilogue reads freed memory, and co_start() dereferences the NULL it finds in tctx->nt -- SEGV at 0xc8, the offset of dead_co in struct rb_native_thread. The removal cannot simply move after the handoff: the GC's root scan walks r->threads.set without the Ractor lock (relying on the barrier), so the unlink must stay where no walker can run -- while th is still registered and before a successor is designated. Nor can the th accesses move before the removal: deregistration itself writes th's running-list node. Instead, keep the removal where it is and give the dying thread an explicit root: r->threads.dying_th, marked by the root scan exactly like a set member, set just before the removal and cleared inside the scheduler-lock section after the epilogue's last use of th. Clearing under the lock serialises the epilogue against its successor -- even a dedicated one woken by to_dead_common() first runs thread_sched_to_running(), which takes the same lock -- so successive epilogues cannot overwrite each other's slot, and the successor cannot publish threads.running_ec before the dying thread has cleared it. Nothing reads th after the clear: the final unlock avoids the debug-logging wrapper (which reads th->serial), and whether the designated successor needs the Ractor enqueued is decided while still holding the lock, since a dedicated successor may run -- and die, freeing itself -- the moment the lock is released. The atfork paths reset the slot so a fork taken mid-epilogue cannot leak a stale root into the child. The slot is accessed with atomic ops: the root scan may read it while another Ractor's dying thread writes it. Also initialise tctx->nt, which native_thread_create_shared() left holding whatever ruby_xmalloc() returned. Not addressed, pre-existing on master: rb_postponed_job_trigger_for_ractor() can copy threads.running_ec and dereference it arbitrarily later (the new root narrows this but cannot protect an already-copied pointer), and a freshly created dedicated native thread publishes its EC via ruby_thread_set_native() before taking the scheduler lock. Two Ractors looping over Thread.new and GC.start crash master in about three seconds (19 of 20 runs); an ASAN build reports the heap-use-after-free directly, at the read of th->nt in coroutine_thread_terminated(), freed by gc_sweep via rb_thread_free_body. With this change ASAN is clean (0/10 vs 7/10 interleaved), as are the reproducer, s7_nested, both branches of the epilogue under stress, a Ractor/GC soak, btest and test-all on release and RUBY_DEBUG=1 builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )