fix(extensions): release saverMu on panic in state store

Extract a runSaver helper that locks saverMu and defers Unlock before
invoking the persistence callback. Without the deferred Unlock, a panic
inside the saver (e.g. disk full mid-write) would leave saverMu held
forever and deadlock the next SetState/DeleteState. Both SetState and
DeleteState now route through the helper. New TestRunner_State_Saver
PanicReleasesSaverMu reproduces the deadlock window with a 2s deadline
and proves the mutex is released after a panic.
This commit is contained in:
Ed Zynda
2026-06-09 15:19:25 +03:00
parent 4a7e4223e0
commit fbf074b1d6
2 changed files with 58 additions and 9 deletions
+17 -9
View File
@@ -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
+41
View File
@@ -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)
}
}