Add the ENV variable for Ractor.check_isolation - #1040
hmcguire-shopify wants to merge 23 commits into
Conversation
946b208 to
9094720
Compare
Sol's Review
|
Run the block in a real non-main Ractor while preserving its closure and argument identities. Downgrade isolation violations to categorized warnings so applications can sweep worker-Ractor compatibility without stopping at the first failure. Support an exclusive scheduler mode for race-free checks and cover the isolation gates, fast paths, messaging, and thread inheritance.
9094720 to
1691eab
Compare
Trigger the isolation check from a boolean environment variable read once at boot instead of from a method call, so an application can be swept for worker-Ractor incompatibilities without editing every Ractor.new call site. Under the variable every non-main Ractor behaves the way check_isolation did: the Proc is not isolated, arguments and the return value pass by reference, and violations are downgraded to :ractor_isolation warnings. Read the variable the way RUBY_RACTOR_EXCLUSIVE is read and relocate the one-shot advisory to boot, keeping its suppression under exclusive mode. Replace the per-Ractor isolation_check field with the process-wide flag. The three sites in thread.c run in the parent's context while creating the child, so they test the flag directly rather than the shared predicate, which evaluates the current Ractor and would never fire from the main one. Because the flag is process-wide it also covers nested Ractors, which previously reverted to raising, so the test asserting that is inverted and a companion pins that Ractor.new still raises when the variable is unset.
Downgrading the isolation violation to a warning left rb_fork_ruby falling through into the fork it had just reported, because rb_raise is NORETURN and rb_ractor_isolation_violation is not. A sweep of an application that calls fork therefore forked, where the same application previously raised Ractor::IsolationError. fork keeps only the calling thread, so the child inherited a VM whose other Ractors' threads were gone while their objspaces were still mapped. Refuse the fork after warning and report it as a failed one: proc_fork_pid turns -1 into rb_sys_fail, rb_daemon returns -1, and the --help pager stops paging.
RUBY_RACTOR_EXCLUSIVE only engages when USE_MN_THREADS is set, so on a non-MN build the advisory correctly still prints and the unconditional zero-advisory assertion would fail. Branch on the +MN marker in RUBY_DESCRIPTION, asserting suppression where exclusive engages and advisory presence where it cannot.
The fall-through to rb_proc_ractor_make_shareable re-reports the same violation, warned in check mode, so the upstream report is skipped there to avoid warning twice. Flagged as suspicious by two review passes; document it at the site.
The advisory announces a mode that changes process behavior, so it must not be silenced by verbosity flags. Kernel.warn in the deleted method form printed under -W0; the relocated rb_warn did not. Use fprintf.
Under RUBY_RACTOR_CHECK_ISOLATION a hot violation warns on every hit, burying the sweep output. Key each warning on the violation's format string and Ruby call site, print the first, and count the rest into a one-line summary at process exit. The raising path is unchanged.
…_EXCLUSIVE The requirements for the isolation-checking mode have two halves: setting the environment variable turns every Ractor::IsolationError into a warning, and it puts all Ractors on the same GVL so the by-reference sharing that check mode permits is not also a data race. Until now the branch delivered only the first half on its own. The second half required a separate RUBY_RACTOR_EXCLUSIVE=1, and a boot advisory nagged whenever it was absent. The second variable existed because the mode used to be entered at runtime through Ractor.check_isolation. A method call cannot retroactively pin the scheduler once the VM has gone multi-Ractor, so serialization had to be requested up front by other means. With the method gone and the mode read in ruby_mn_threads_params, that constraint no longer holds: boot precedes the first Ractor, so check mode can arrange the scheduler itself. Derive the exclusive flag from ruby_ractor_check_isolation_enabled and leave the existing mechanism (force M:N on, clamp max_cpu to 1) exactly as it was. This removes ractor_exclusive_env_p, the ruby_ractor_exclusive_enabled global and its extern; no reference to RUBY_RACTOR_EXCLUSIVE remains. The variable was never documented or upstreamed and had been carried between unmerged branches, so nothing outside this branch depends on it. The boot advisory changes meaning. It used to key on the absence of the env var, which was wrong in both directions: it fired when the user had already serialized by hand with RUBY_MN_THREADS=1 RUBY_MAX_CPU=1, and it recommended a variable that did nothing on builds without M:N. It now fires only when serialization genuinely could not be arranged, that is on a build compiled without M:N support, and says so. Measured with two CPU-bound non-main Ractors: with the variable set they run back to back with zero overlap; unset they run fully in parallel. The default path is unchanged. Note that max_cpu = 1 serializes M:N-scheduled threads only; Ractors that take dedicated native threads can still overlap, and this does nothing about cross-objspace GC. It reduces the concurrent mutation risk of by-reference sharing rather than eliminating it. Tests: the advisory test now asserts absence on M:N builds and presence otherwise; the serialization test needs only RUBY_RACTOR_CHECK_ISOLATION=1. Two comments that pointed at the removed variable are reworded.
|
Setting There is no method to call anymore. What the mode does
MoreGC is global in check mode. Check mode passes arguments, block values and messages between Ractors by reference, so a Ractor's heap can be reachable only from another Ractor's roots. Every collection is now promoted to a global cycle over all objspaces, and a dying Ractor skips its local retire GC and leaves its pages for the next global sweep. This closes review finding #2 (receiver holding a sender's object past the sender's GC) and the related "try to mark T_NONE" crash from finding #1. Two regression tests cover both directions.
Grouping is keyed on the app site. The outer-variable Proc warning and the by-reference copy warning bypassed the dedup and reported Example: three workers, each capturing an outer variable and sending five unshareable arrays. x = [1]
port = Ractor::Port.new
3.times { Ractor.new(port) { |pt| 5.times { pt << [x] } }.value }Level 1 groups the 18 hits into 2 lines: Level 2 leaves the grouping to the consumer: |
The branch carried about three times the comment density of the code around it. Most of the excess fell into three patterns: restating the mode's rationale at every call site, noting that check-mode Ractors "also route here" through guards that already cover every non-main Ractor, and explaining why unchanged code was left unchanged. Two externs were the only commented externs in their files, and the mode description was repeated in ractor.c, ractor_core.h and version.c. Keep one description at the definition site, cut each remaining block to the single fact the code does not show, and drop the rest. No behavioral change.
Master moved the Ractor isolation checks from "is this the main Ractor" to class ownership (rb_class_owned_p) and turned RUBY_MN_THREADS into a mode integer. The conflicts are resolved by keeping master's conditions and messages and routing them through rb_ractor_isolation_violation, so check mode still downgrades them to warnings: - variable.c, vm_insnhelper.c: ownership-based ivar/cvar/constant checks warn in check mode. Class names are still formatted with rb_class_path, since a user to_s could re-enter the same check now that it can return. - variable.c: rb_gvar_get/rb_gvar_set used to skip the lookup work and raise after releasing the VM lock. In check mode the entry is still created and the access proceeds after the warning; otherwise it dereferenced NULL. - class.c: rb_class_owner_check (def, include, define_method, ... on a class owned by another Ractor) goes through the same warn-or-raise path, so a check-mode sweep survives reopening a main-Ractor class. - thread_sched.c: check mode raises RUBY_MN_THREADS to at least 1 and still clamps RUBY_MAX_CPU to 1; an explicit =2 is kept.
rb_ractor_confirm_belonging (RACTOR_CHECK_MODE, i.e. RUBY_DEBUG or VM_CHECK_MODE builds) aborts when an unshareable object of another Ractor's objspace is pushed on the VM stack. Check mode does exactly that on purpose: Procs are not isolated and arguments and return values travel by reference. Five test_ractor tests failed on every Ubuntu CI job for that reason while passing on the non-debug macOS and Windows jobs.
Level 1 keeps the per-site dedup and exit summary for reading stderr by eye. Level 2 emits every hit so an external collector can count them and record where each was first seen.
Check mode passes arguments, block values and messages between Ractors by reference, so a Ractor's heap can be reachable only from another Ractor's roots. A local cycle cannot see those edges and frees live objects. Treat the process as one logical heap: promote every collection to a global cycle and skip the dying Ractor's local retire GC, leaving its pages for the next global sweep.
The outer-variable Proc warning and the by-reference copy warning called rb_category_warn directly, so level 1 printed them on every hit and reported <internal:ractor> as the site. Route every warning through one emitter that keys the dedup on the message and the nearest non-internal Ruby frame, and reports that frame.
Skip formatting disabled isolation warnings and allocate deduplication keys only for new message/source combinations. Keep the source and message strings rooted while consulting the warning table. Reuse the ordinary thread argument path for isolation-check Ractors and share warning capture setup across the isolation tests. Add coverage for deduplication across messages and source files. Validated with the Ractor, Thread, and Ruby option suites: 254 tests, 3712 assertions, no failures or errors, and four skips. Also checked deduplication across forced GC compaction.
Report class copy, attached-object and Proc instance-variable violations in diagnostic mode. Preserve requested Proc receivers without inheriting shareability, and prevent constant caches from bypassing isolation warnings. Add regression coverage for diagnostic and normal isolation behavior, including constant reads under YJIT and ZJIT.
Make strict and warning-only Proc checks explicit, simplify warning control flow, and consolidate method-call isolation checks. Remove an unnecessary Proc-copy wrapper while preserving behavior.
There was a problem hiding this comment.
Things look very good in general 👍
One thing I noticed was that GC.verify_internal_consistency is broken:
p Ractor.new { GC.verify_internal_consistency; :ok }.value
=begin
$ RUBY_RACTOR_ISOLATION=1 ./ruby -W0 verify_iso.rb
check_children_i: containment violation: unshareable VM/thread (objspace 0x94b0b1000) -> foreign unshareable proc (objspace 0x106976b40)
check_children_i: containment violation: unshareable fiber (objspace 0x94b0b1000) -> foreign unshareable T_IMEMO (objspace 0x106976b40)
root_scope_check_i: root category "ractor" names a foreign unshareable without a shref record: proc (owner 0x106976b40, self 0x94b0b1000)
-e:1: [BUG] gc_verify_internal_consistency: found internal inconsistency.
...
-e:1: [BUG] Segmentation fault at 0xfffffffffffffff8
=end
You just need to skip these checks in check isolation mode.
| const struct isolation_warn_key *key1 = (const void *)a; | ||
| const struct isolation_warn_key *key2 = (const void *)b; | ||
| return key1->line != key2->line || | ||
| strcmp(key1->file, key2->file) || strcmp(key1->message, key2->message); |
There was a problem hiding this comment.
I'm guessing this is defeated for any isolation error that shows the object's address. Is that why they try to avoid that by showing the class name? Do all the messages do this or only some? Either way, seems okay.
|
@luke-gruber thanks! Will do changes. |
Run the block in a real non-main Ractor while preserving its closure and argument identities. Downgrade isolation violations to categorized warnings so applications can sweep worker-Ractor compatibility without stopping at the first failure.
Support an exclusive scheduler mode for race-free checks and cover the isolation gates, fast paths, messaging, and thread inheritance.