Skip to content

Commit d3e4661

Browse files
ovaistariqclaude
andauthored
fix: Prevent deadlock in loadListing unlock/relock pattern (#43)
* fix: Prevent deadlock in loadListing unlock/relock pattern This fixes potential deadlocks in the loadListing() function where unlock/relock patterns could cause circular lock dependencies. The deadlock could occur in two places: 1. When calling slurpOnce() at line 718-725 2. When calling listObjectsFlat() at line 737-742 Both cases released parent.mu (and dh.mu in case 1) while other goroutines could be holding different locks and waiting, creating circular dependencies. The solution uses the generation counter approach from PR #39: - Check generation before unlocking - After relocking, detect if generation changed - If changed, reset directory handle position This ensures proper synchronization without risking deadlocks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 97e8dc9 commit d3e4661

File tree

1 file changed

+22
-0
lines changed

1 file changed

+22
-0
lines changed

core/dir.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,11 +716,21 @@ func (dh *DirHandle) loadListing() error {
716716
// token
717717

718718
if useSlurp {
719+
// We must release both locks before calling slurpOnce to avoid deadlock
720+
// Store current generation before unlocking
721+
currentGen := atomic.LoadUint64(&parent.dir.generation)
719722
parent.mu.Unlock()
720723
dh.mu.Unlock()
721724
done, err := parent.slurpOnce(true)
722725
dh.mu.Lock()
723726
parent.mu.Lock()
727+
// Check if generation changed while we were unlocked
728+
newGen := atomic.LoadUint64(&parent.dir.generation)
729+
if newGen != currentGen {
730+
// Directory structure changed, reset our position
731+
dh.generation = newGen
732+
dh.lastInternalOffset = -1
733+
}
724734
if err != nil {
725735
return err
726736
}
@@ -734,12 +744,24 @@ func (dh *DirHandle) loadListing() error {
734744

735745
loaded, startMarker := false, ""
736746
for parent.dir.lastFromCloud == nil && !parent.dir.listDone {
747+
// We must release parent.mu before calling listObjectsFlat to avoid deadlock
748+
// Store current generation before unlocking
749+
currentGen := atomic.LoadUint64(&parent.dir.generation)
737750
parent.mu.Unlock()
738751
start, err := dh.listObjectsFlat()
739752
if !loaded {
740753
loaded, startMarker = true, start
741754
}
742755
parent.mu.Lock()
756+
// Check if generation changed while we were unlocked
757+
newGen := atomic.LoadUint64(&parent.dir.generation)
758+
if newGen != currentGen {
759+
// Directory structure changed, reset our position and invalidate startMarker
760+
dh.generation = newGen
761+
dh.lastInternalOffset = -1
762+
// Clear startMarker to prevent removeExpired from operating on stale range
763+
startMarker = ""
764+
}
743765
if err != nil {
744766
return err
745767
}

0 commit comments

Comments
 (0)