From b9bb061f49e6b19f5bbd98089fd7aa79222c98be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20H=C3=A4cker?= Date: Sat, 11 Jul 2026 08:35:40 +0200 Subject: [PATCH 1/2] abstractinterpret: Fix data race on shared `binding_states` `report_package` analyzes method signatures concurrently via `Threads.@spawn` (aviatesk/JET.jl#785). Each task gets its own analyzer with refreshed local caches, but every task still shares a single `binding_states::IdDict` held in `AnalyzerState`. That map is mutated during inference whenever a method assigns to a global (e.g. in `global_assignment_rt_exct`), so concurrent `setindex!`/rehash corrupts it. It typically surfaces later as: UndefRefError: access to undefined reference from the `unique!` over the collected reports. The shared write itself dates back to aviatesk/JET.jl#698, but it was benign while `report_package` was single-threaded; aviatesk/JET.jl#785 made the analysis concurrent without isolating or guarding it. Binding partitions are a Julia 1.12 feature, so this affects both 1.12 and 1.13. Wrap `binding_states` in an `AbstractBindings` type that pairs the `IdDict` with a `ReentrantLock` and forwards `getindex`, `setindex!`, `haskey`, and `get` through it. The wrapper is also made lockable so the read-modify-write in `const_assignment_rt_exct` can run under one `@lock` and stay atomic. This keeps the shared-and-reused design intact (no per-task isolation, parallelism preserved) rather than giving each task a private map. Accesses are infrequent (global assignments and partition lookups) and the lock is uncontended in the common case, so the overhead is negligible. `test_typeinfer` and `test_virtualprocess` pass. I used Claude Opus 4.8 while implementing this. --- src/abstractinterpret/abstractanalyzer.jl | 24 +++++++++++++++++++++- src/abstractinterpret/typeinfer.jl | 25 +++++++++++++---------- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/src/abstractinterpret/abstractanalyzer.jl b/src/abstractinterpret/abstractanalyzer.jl index 696117a2c..fb763bf37 100644 --- a/src/abstractinterpret/abstractanalyzer.jl +++ b/src/abstractinterpret/abstractanalyzer.jl @@ -84,7 +84,29 @@ struct AbstractBindingState AbstractBindingState(isconst::Bool, maybeundef::Bool, @nospecialize typ) = new(isconst, maybeundef, typ) AbstractBindingState(isconst::Bool, maybeundef::Bool) = new(isconst, maybeundef) end -const AbstractBindings = IdDict{Core.BindingPartition,AbstractBindingState} + +# `report_package` analyzes signatures concurrently while all task analyzers share a single +# `binding_states::AbstractBindings` (see `AnalyzerState`). Guard the underlying `IdDict` with +# a lock so concurrent `setindex!`/rehash cannot corrupt it; the accesses are infrequent +# (only on global binding assignments and binding-partition lookups), so the uncontended lock +# cost is negligible. +struct AbstractBindings + bindings::IdDict{Core.BindingPartition,AbstractBindingState} + lock::ReentrantLock + AbstractBindings() = new(IdDict{Core.BindingPartition,AbstractBindingState}(), ReentrantLock()) +end +@inline Base.getindex(b::AbstractBindings, partition::Core.BindingPartition) = + @lock b.lock b.bindings[partition] +@inline Base.setindex!(b::AbstractBindings, binding_state::AbstractBindingState, partition::Core.BindingPartition) = + (@lock b.lock b.bindings[partition] = binding_state; return b) +@inline Base.haskey(b::AbstractBindings, partition::Core.BindingPartition) = + @lock b.lock haskey(b.bindings, partition) +@inline Base.get(b::AbstractBindings, partition::Core.BindingPartition, @nospecialize(default)) = + @lock b.lock get(b.bindings, partition, default) +# Make `AbstractBindings` usable as a lock so callers can perform atomic +# read-modify-write updates (the wrapper's own accessors re-enter safely). +Base.lock(b::AbstractBindings) = lock(b.lock) +Base.unlock(b::AbstractBindings) = unlock(b.lock) """ mutable struct AnalyzerState diff --git a/src/abstractinterpret/typeinfer.jl b/src/abstractinterpret/typeinfer.jl index 032ffdf1e..9e2ebef92 100644 --- a/src/abstractinterpret/typeinfer.jl +++ b/src/abstractinterpret/typeinfer.jl @@ -558,18 +558,21 @@ function const_assignment_rt_exct(analyzer::ToplevelAbstractAnalyzer, sv::Infere if rt !== Union{} # `:const` assignment destructively overrides the binding type binding_states = get_binding_states(analyzer) - if !isconditional - binding_state = AbstractBindingState(true, false, new_binding_typ′[]) - elseif haskey(binding_states, partition) - old_binding_state = binding_states[partition] - @assert old_binding_state.isconst && isdefined(old_binding_state, :typ) - newmaybeundef = old_binding_state.maybeundef & isconditional - newtyp = old_binding_state.typ ⊔ new_binding_typ′[] - binding_state = AbstractBindingState(true, newmaybeundef, newtyp) - else - binding_state = AbstractBindingState(true, true, new_binding_typ′[]) + binding_state = @lock binding_states begin + if !isconditional + new_state = AbstractBindingState(true, false, new_binding_typ′[]) + elseif haskey(binding_states, partition) + old_binding_state = binding_states[partition] + @assert old_binding_state.isconst && isdefined(old_binding_state, :typ) + newmaybeundef = old_binding_state.maybeundef & isconditional + newtyp = old_binding_state.typ ⊔ new_binding_typ′[] + new_state = AbstractBindingState(true, newmaybeundef, newtyp) + else + new_state = AbstractBindingState(true, true, new_binding_typ′[]) + end + binding_states[partition] = new_state + new_state end - binding_states[partition] = binding_state # HACK/FIXME Concretize `AbstractBindingState` # For top-level analysis implementation reasons, we actually define this # `AbstractBindingState` in the analyzed module’s namespace. From 4bac7f49ac7be1b190331e4febad7a4f12faaaed Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Sun, 19 Jul 2026 23:07:19 +0900 Subject: [PATCH 2/2] Address reviews --- src/abstractinterpret/abstractanalyzer.jl | 4 ---- src/abstractinterpret/typeinfer.jl | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/abstractinterpret/abstractanalyzer.jl b/src/abstractinterpret/abstractanalyzer.jl index fb763bf37..aa115cbf6 100644 --- a/src/abstractinterpret/abstractanalyzer.jl +++ b/src/abstractinterpret/abstractanalyzer.jl @@ -103,10 +103,6 @@ end @lock b.lock haskey(b.bindings, partition) @inline Base.get(b::AbstractBindings, partition::Core.BindingPartition, @nospecialize(default)) = @lock b.lock get(b.bindings, partition, default) -# Make `AbstractBindings` usable as a lock so callers can perform atomic -# read-modify-write updates (the wrapper's own accessors re-enter safely). -Base.lock(b::AbstractBindings) = lock(b.lock) -Base.unlock(b::AbstractBindings) = unlock(b.lock) """ mutable struct AnalyzerState diff --git a/src/abstractinterpret/typeinfer.jl b/src/abstractinterpret/typeinfer.jl index 9e2ebef92..44bcfbc65 100644 --- a/src/abstractinterpret/typeinfer.jl +++ b/src/abstractinterpret/typeinfer.jl @@ -558,7 +558,7 @@ function const_assignment_rt_exct(analyzer::ToplevelAbstractAnalyzer, sv::Infere if rt !== Union{} # `:const` assignment destructively overrides the binding type binding_states = get_binding_states(analyzer) - binding_state = @lock binding_states begin + binding_state = @lock binding_states.lock begin if !isconditional new_state = AbstractBindingState(true, false, new_binding_typ′[]) elseif haskey(binding_states, partition)