Fix double-unlock panic in ProcessUniqueCollision (multi-UNIQUE tables)#12
Open
Fix double-unlock panic in ProcessUniqueCollision (multi-UNIQUE tables)#12
Conversation
…E constraints Root cause: when a table has multiple UNIQUE constraints (e.g. PK + UNIQUE email), ProcessUniqueCollision is called recursively with idx+1. The outer call (idx=0) holds t.uniquelock. The inner call (idx=1) does not hold the lock itself, but its collision-handler path unconditionally calls lock.Unlock() (line 1107), re-acquires it in the inner defer on panic, then releases it again in the outer defer. When the panic propagates back to idx=0, its outer defer also tried to unlock t.uniquelock → fatal "sync: unlock of unlocked mutex", crashing the server. Fix: wrap both recursive ProcessUniqueCollision calls (intermediate and final flush) with a recovery closure that sets uniquelockHeld=false before re-panicking. This tells the outer defer that the lock was already released by the inner call's defer chain, so it must not unlock again. The non-panic path (UPSERT success) is unaffected: the inner call re-acquires the lock at line 1135 and returns normally; the outer caller releases it at line 1153. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
carli2
added a commit
that referenced
this pull request
Mar 3, 2026
Cherry-pick 6b8f3a4: when a table has PK + UNIQUE email, the recursive ProcessUniqueCollision call (idx=1) was unconditionally unlocking t.uniquelock held by idx=0, then idx=0's defer also unlocked → fatal double-unlock crash. Fix: wrap recursive flush calls in recovery closures that clear uniquelockHeld=false before re-panicking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
id PRIMARY KEY, email UNIQUE) crash the server withfatal: sync: unlock of unlocked mutexwhen a constraint violation is detected in the second constraint (idx=1).ProcessUniqueCollisionis recursive — idx=0 holdst.uniquelock, calls idx=1 which does not own the lock. On a panic infailure(), idx=1's defer chain releases the lock, then idx=0's outer defer tries to release it again → fatal double-unlock, server crash.uniquelockHeld=falsebefore re-panicking, so idx=0's outer defer knows the lock was already released.Test plan
tests/92_unique_main_delta.yaml— 45/45 (was crashing the server before)tests/29_mysql_upsert.yaml— 25/25 (UPSERT on secondary UNIQUE keys)tests/33_collations_order.yaml— 4/4 (was failing as secondary victim of the server crash)🤖 Generated with Claude Code