fix: unlock l.mu before OnPotentialDeadlock to support panic in callback

Copy stacks into l.order while l.mu is still held, before unlocking
for the OnPotentialDeadlock callback. This prevents a data race where
concurrent postUnlock calls can recycle the pooled stack buffer
(bs.stack) during the unlock window, causing copyStack to race with
pool writers.

For order violations: print diagnostics, copy stacks, store in
l.order, THEN unlock and call OnPotentialDeadlock.
For non-violations: copy stacks and store while l.mu is held (no
unlock needed).

Also fixes the duplicate-lock detection path to unlock l.mu before
calling OnPotentialDeadlock, allowing the callback to panic without
leaving the mutex locked.

Author: Yanhu007

deadlock.go

@@ -427,8 +427,9 @@ func (l *lockOrder) preLock(stack []uintptr, p interface{}) {
 						buf.Flush()
 					}
 					Opts.mu.Unlock()
+					l.mu.Unlock()
 					Opts.OnPotentialDeadlock()
-					break
+					return
 				}
 			}
 			continue
@@ -454,11 +455,18 @@ func (l *lockOrder) preLock(stack []uintptr, p interface{}) {
 					buf.Flush()
 				}
 				Opts.mu.Unlock()
+				// Copy stacks while l.mu is still held, BEFORE unlocking for the
+				// callback. The pooled buffers backing bs.stack can be recycled by
+				// concurrent postUnlock calls during the unlock window.
+				l.order[newBeforeAfter(b, p)] = ss{copyStack(bs.stack), copyStack(stack)}
+				l.mu.Unlock()
 				Opts.OnPotentialDeadlock()
+				l.mu.Lock()
+			} else {
+				// No violation — still need to record the ordering.
+				// l.mu is held, so copyStack is safe here.
+				l.order[newBeforeAfter(b, p)] = ss{copyStack(bs.stack), copyStack(stack)}
 			}
-			// Copy both stacks: they're backed by pooled buffers that will be
-			// recycled in postUnlock, but l.order entries persist until MaxMapSize.
-			l.order[newBeforeAfter(b, p)] = ss{copyStack(bs.stack), copyStack(stack)}
 			if len(l.order) == Opts.MaxMapSize { // Reset the map to keep memory footprint bounded.
 				l.order = map[beforeAfter]ss{}
 			}

deadlock_test.go

@@ -393,3 +393,82 @@ func TestLockDuplicate(t *testing.T) {
 		t.Fatalf("expected 2 deadlocks, detected %d", deadlocks)
 	}
 }
+
+// TestOrderViolationCallbackPooledStackRace demonstrates a data race introduced
+// by PR #55's order-violation path: preLock unlocks lo.mu before calling
+// OnPotentialDeadlock and re-acquires it after, but then continues to read
+// bs.stack — a slice into a pooled stack buffer — and copyStack() it into
+// lo.order. During the unlock window, a concurrent cross-goroutine Unlock can
+// release that exact buffer back to stackBufPool, where another goroutine grabs
+// it from the pool and writes to it. The subsequent copyStack(bs.stack) then
+// races with that write.
+//
+// Run with `go test -race -run TestOrderViolationCallbackPooledStackRace` to
+// observe the data race report.
+func TestOrderViolationCallbackPooledStackRace(t *testing.T) {
+	defer restore()()
+	Opts.DeadlockTimeout = 0
+	Opts.OnPotentialDeadlock = func() {} // placeholder, overridden below
+
+	var a, b Mutex
+
+	// Prime lo.order with the "a then b" ordering.
+	a.Lock()
+	b.Lock()
+	b.Unlock()
+	a.Unlock()
+
+	// Hold b in this goroutine, then snapshot the exact stack buffer that
+	// preLock will later read through bs.stack. We close over this pointer in
+	// the helper to write to it concurrently with preLock's copyStack().
+	b.Lock()
+
+	lo.mu.Lock()
+	if len(lo.cur[&b]) != 1 {
+		lo.mu.Unlock()
+		t.Fatalf("expected exactly 1 holder for b, got %d", len(lo.cur[&b]))
+	}
+	holderBuf := lo.cur[&b][0].buf
+	lo.mu.Unlock()
+
+	if holderBuf == nil {
+		t.Fatal("expected holder buf to be non-nil")
+	}
+
+	callbackEntered := make(chan struct{})
+	helperDone := make(chan struct{})
+
+	Opts.OnPotentialDeadlock = func() {
+		// At this point in the post-PR code, lo.mu has already been released.
+		// Signal the helper to start writing to the pooled buffer.
+		close(callbackEntered)
+		// Give the helper time to start churning so its writes overlap with
+		// preLock's copyStack(bs.stack) after this callback returns and lo.mu
+		// is re-acquired.
+		time.Sleep(50 * time.Millisecond)
+	}
+
+	go func() {
+		defer close(helperDone)
+		<-callbackEntered
+		// Tight write loop directly on the same backing array that preLock's
+		// bs.stack points into. preLock will re-acquire lo.mu after the
+		// callback and execute copyStack(bs.stack), which calls
+		// copy(c, bs.stack) — a read of this exact memory. Since the writes
+		// here have no happens-before with that read (no shared mutex, no
+		// channel), the race detector will flag the read.
+		for i := 0; i < 5000000; i++ {
+			holderBuf[0] = uintptr(i)
+		}
+	}()
+
+	// Trigger preLock's order-violation path. Post-PR this calls the callback,
+	// re-acquires lo.mu, and runs copyStack(bs.stack) — racing the helper.
+	a.Lock()
+	a.Unlock()
+
+	<-helperDone
+
+	// b is still locked (we never unlocked it). Release it for cleanup.
+	b.Unlock()
+}