From 3b6856904f6025763cda8bf325c47ee5a3e7c56f Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 2 Nov 2025 18:16:25 -0800 Subject: [PATCH] Fix critical double-release race in importInProgress flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL BUG: The previous fix had a race condition where the importInProgress flag could be released twice, allowing two goroutines to think they both hold the lock. Bug scenario: 1. Goroutine A: acquires lock (CAS true) 2. Goroutine A: manually releases at line 208 for git dirty skip 3. Goroutine B: CAS succeeds, acquires lock 4. Goroutine A: defer runs, releases flag AGAIN (clears B lock) 5. Goroutine C: CAS succeeds - now TWO goroutines have lock Root cause: Using both manual Store(false) AND defer Store(false) created a window where the flag could be cleared twice. Fix: Use a shouldDeferRelease flag to disable the deferred release when we manually release early. This ensures exactly one release per acquisition. Testing: All auto-import tests still passing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .beads/daemon-stderr.log | 2 ++ internal/rpc/server_export_import_auto.go | 13 +++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 .beads/daemon-stderr.log diff --git a/.beads/daemon-stderr.log b/.beads/daemon-stderr.log new file mode 100644 index 00000000..66858901 --- /dev/null +++ b/.beads/daemon-stderr.log @@ -0,0 +1,2 @@ +Error: daemon already running (PID 27867) +Use 'bd daemon --stop' to stop it first diff --git a/internal/rpc/server_export_import_auto.go b/internal/rpc/server_export_import_auto.go index 1d6a3579..8ce0d414 100644 --- a/internal/rpc/server_export_import_auto.go +++ b/internal/rpc/server_export_import_auto.go @@ -196,7 +196,15 @@ func (s *Server) checkAndAutoImportIfStale(req *Request) error { } return nil } - defer s.importInProgress.Store(false) + + // Track whether we should release the lock via defer + // Set to false if we manually release early to avoid double-release bug + shouldDeferRelease := true + defer func() { + if shouldDeferRelease { + s.importInProgress.Store(false) + } + }() // Check if git has uncommitted changes that include beads files (bd-8931) // If JSONL files are uncommitted, skip auto-import to avoid conflicts @@ -204,8 +212,9 @@ func (s *Server) checkAndAutoImportIfStale(req *Request) error { dbDir := filepath.Dir(dbPath) workspaceRoot := filepath.Dir(dbDir) // Go up from .beads to workspace root if hasUncommittedBeadsFiles(workspaceRoot) { - // CRITICAL: Must release lock before returning to avoid race condition + // CRITICAL: Release lock and disable defer to avoid double-release race s.importInProgress.Store(false) + shouldDeferRelease = false if os.Getenv("BD_DEBUG") != "" { fmt.Fprintf(os.Stderr, "Debug: skipping auto-import, .beads files have uncommitted changes\n")