Serialize OpenBucket and stop expiration before closing the bucket

[torcolvin]

Two OpenBucket calls for the same bucket could both miss the cache, so both opened SQLite and both registered. This caused these bugs:

• The call that lost the race called Close, which lowered the registry count for the copy that it returned.
• A CreateNew call that lost the race returned the other call's bucket with no error, instead of fs.ErrExist.
• _scheduleExpiration ran without the expiry manager lock, while writes (feeds.go:179) scheduled expiration at the same time. This was a data race.

Fix: OpenBucket now holds openLock on the registry from getCachedBucket until it registers the bucket. Only one call opens a bucket, so no call can lose the race. The bucket schedules expiration before it is registered, and it takes the expiry manager lock to do so.

Close and expiration also locked two mutexes in opposite orders, so they could deadlock:

• close() locked bucket.mutex, then expManager.mutex.
• runExpiry locked expManager.mutex, then bucket.mutex.

Fix:

• close() now stops the expiry manager before it locks the bucket.
• stop() waits for a running expiration to finish.
• runExpiry holds a separate runMutex, so no expiration can start after stop().

A new CI job builds with -tags deadlock. That build uses go-deadlock mutexes, and the job fails on any lock order inversion, even when the goroutines do not deadlock during the test.

Tests:

• TestConcurrentOpenBucket checks that concurrent reopens open the database one time.
• TestConcurrentCreateNewBucket checks that one CreateNew call succeeds and the others get fs.ErrExist.
• TestCloseDuringExpiration checks that Close waits for a running expiration. The old code deadlocked in this test.

I ran the rosmar tests 200 times with -race, and 150 times with -race -tags deadlock. I also ran the sync gateway tests with this set of changes, with and without -race.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

Resolve the lock-order deadlock and make the registration-race test deterministic.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Avoid deadlock from inconsistent expiration and bucket lock ordering · New
• Make registration-loser test deterministic instead of silently skipping · New

What changed in this PR

Fixes OpenBucket reference counting and expiration scheduling during concurrent registration races.

Changes:

• Corrects loser-bucket cleanup without unregistering.
• Synchronizes expiration scheduling.
• Adds concurrent-open regression coverage.

File	Summary
bucket.go	Handles registration-race losers safely.
bucket_registry_test.go	Adds concurrent bucket-opening coverage; the race path should be made deterministic.
bucket_api.go	Adds expiration scheduling synchronization; a lock-order deadlock must be resolved.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Fix CI test-status propagation and address unnecessary serialization of unrelated bucket opens.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Pipeline masks go test failures · New

Resolved since last review (2)

• Avoid deadlock from inconsistent expiration and bucket lock ordering
• Make registration-loser test deterministic instead of silently skipping