diff --git a/internal/extensions/runner.go b/internal/extensions/runner.go index 1564e0b4..a48888a6 100644 --- a/internal/extensions/runner.go +++ b/internal/extensions/runner.go @@ -784,11 +784,7 @@ func (r *Runner) SetState(key, value string) { r.state[key] = value saver := r.stateSaver r.stateMu.Unlock() - if saver != nil { - r.saverMu.Lock() - saver() - r.saverMu.Unlock() - } + r.runSaver(saver) } // GetState returns the value previously stored via SetState, plus a bool @@ -811,11 +807,23 @@ func (r *Runner) DeleteState(key string) { } saver := r.stateSaver r.stateMu.Unlock() - if existed && saver != nil { - r.saverMu.Lock() - saver() - r.saverMu.Unlock() + if !existed { + return } + r.runSaver(saver) +} + +// runSaver invokes the optional persistence callback under saverMu so +// concurrent SetState/DeleteState writers cannot race on the shared tmp +// file used by SaveStateToFile's atomic rename. The deferred Unlock +// guarantees saverMu is released even if the saver panics. +func (r *Runner) runSaver(saver func()) { + if saver == nil { + return + } + r.saverMu.Lock() + defer r.saverMu.Unlock() + saver() } // ListState returns all keys currently in the state store, in unspecified diff --git a/internal/extensions/state_test.go b/internal/extensions/state_test.go index c570848e..3dd0c045 100644 --- a/internal/extensions/state_test.go +++ b/internal/extensions/state_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "sync" "testing" + "time" ) func TestRunner_State_BasicSetGetDelete(t *testing.T) { @@ -219,3 +220,43 @@ func TestRunner_State_ContextNoOpsWhenUnset(t *testing.T) { t.Fatalf("emit: %v", err) } } + +func TestRunner_State_SaverPanicReleasesSaverMu(t *testing.T) { + // If the saver callback panics (e.g. disk full mid-write), runSaver + // must still release saverMu so subsequent SetState/DeleteState calls + // can make progress. Without `defer Unlock()` the lock would be + // permanently held and the next write would deadlock. + r := NewRunner(nil) + var calls int + r.SetStateSaver(func() { + calls++ + if calls == 1 { + panic("simulated disk-write failure") + } + }) + + // First call panics. Recover, then verify a follow-up call still works + // without blocking (proving saverMu was released). + func() { + defer func() { + if rec := recover(); rec == nil { + t.Fatal("expected panic from first saver invocation") + } + }() + r.SetState("a", "1") + }() + + done := make(chan struct{}) + go func() { + r.SetState("b", "2") // would deadlock if saverMu were still held + close(done) + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("SetState after saver panic blocked — saverMu was not released") + } + if calls != 2 { + t.Errorf("expected saver to fire twice (panic + recovery write), got %d", calls) + } +}