Skip to content

Fix double mutex unlock in concurrent contract save queue#2133

Merged
mkmccarty merged 3 commits intomm-branch-1from
copilot/sub-pr-2131-again
Jan 30, 2026
Merged

Fix double mutex unlock in concurrent contract save queue#2133
mkmccarty merged 3 commits intomm-branch-1from
copilot/sub-pr-2131-again

Conversation

Copy link
Contributor

Copilot AI commented Jan 30, 2026

Addresses feedback from PR #2131 regarding a mutex double-unlock bug in saveData().

Issue

When the save queue is full, break only exits the select statement, not the for loop, causing execution to fall through to a second unlock:

saveQueueMutex.Lock()
select {
case saveQueue <- contractHash:
    // Successfully queued
default:
    saveQueueMutex.Unlock()  // First unlock
    break                     // Only exits select, not for loop!
}
saveQueueMutex.Unlock()      // Second unlock → panic

Fix

Replace break with continue to skip the second unlock when queue is full:

default:
    saveQueueMutex.Unlock()
    continue  // Skip to next loop iteration

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: mkmccarty <57630+mkmccarty@users.noreply.github.com>
Copilot AI changed the title [WIP] Update implementation based on feedback for contract save queue Fix double mutex unlock in concurrent contract save queue Jan 30, 2026
Copilot AI requested a review from mkmccarty January 30, 2026 01:55
@mkmccarty mkmccarty marked this pull request as ready for review January 30, 2026 02:00
Copilot AI review requested due to automatic review settings January 30, 2026 02:00
@mkmccarty mkmccarty merged commit 276c42e into mm-branch-1 Jan 30, 2026
12 checks passed
@mkmccarty mkmccarty deleted the copilot/sub-pr-2131-again branch January 30, 2026 02:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a concurrency bug in saveData() where the “save queue full” path could fall through and unlock the same mutex twice.

Changes:

  • Replace break with continue in the queue-full default branch to skip the outer unlock.
  • Explicitly unlock saveQueueMutex before continuing to the next loop iteration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 116 to +119
// Queue is full, skip this one (it will be retried in the next save cycle)
log.Printf("Save queue full, skipping contract: %s", contractHash)
// Do not mark as pending so it can be retried in a future save cycle
break
saveQueueMutex.Unlock()
continue
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the default branch, log.Printf runs while saveQueueMutex is held. Logging can block (I/O) and prolong mutex hold time, which may increase contention and delay the worker’s pendingSaves cleanup under load. Consider unlocking before logging (capture contractHash first) and keeping the locked section minimal; optionally rate-limit this message to avoid log spam when the queue is full.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants