Follow-up from the independent review of #2619 (live in-place processor reconfigure).
pkg/processor.Instance.running is now an atomic.Bool (that PR fixed the raw data race). But the logical check-then-act remains non-atomic across method calls:
Service.Update / Service.Delete do if instance.running.Load() { refuse } then mutate/delete.Service.MakeRunnableProcessor does CompareAndSwap(false, true) to reserve.
A concurrent MakeRunnableProcessor (pipeline start) flipping running to true between an Update/Delete guard's Load()==false and its subsequent mutation is a TOCTOU: the guard could pass, then the instance becomes running, and the mutation proceeds against a now-running instance.
This is not introduced by #2619 — the old plain-bool code had the identical structure, and atomic.Bool neither introduces nor worsens it. Whether it's reachable depends on higher-level serialization between pipeline start and API Update (orchestrator/lifecycle layer). Fixing it properly requires holding a lock across the whole build/mutate spans (a per-instance mutex), a larger design change than the atomic swap.
Scope: decide whether the higher layers already serialize these paths (in which case document that invariant), or add a per-instance mutex spanning the guarded operations. Data-path (Tier 1) — needs a design note if we add locking.
Non-blocking for v0.17.
Follow-up from the independent review of #2619 (live in-place processor reconfigure).
pkg/processor.Instance.runningis now anatomic.Bool(that PR fixed the raw data race). But the logical check-then-act remains non-atomic across method calls:Service.Update/Service.Deletedoif instance.running.Load() { refuse }then mutate/delete.Service.MakeRunnableProcessordoesCompareAndSwap(false, true)to reserve.A concurrent
MakeRunnableProcessor(pipeline start) flippingrunningto true between anUpdate/Deleteguard'sLoad()==falseand its subsequent mutation is a TOCTOU: the guard could pass, then the instance becomes running, and the mutation proceeds against a now-running instance.This is not introduced by #2619 — the old plain-
boolcode had the identical structure, andatomic.Boolneither introduces nor worsens it. Whether it's reachable depends on higher-level serialization between pipeline start and APIUpdate(orchestrator/lifecycle layer). Fixing it properly requires holding a lock across the whole build/mutate spans (a per-instance mutex), a larger design change than the atomic swap.Scope: decide whether the higher layers already serialize these paths (in which case document that invariant), or add a per-instance mutex spanning the guarded operations. Data-path (Tier 1) — needs a design note if we add locking.
Non-blocking for v0.17.