cockroachdb / pebble

RocksDB/LevelDB inspired key-value database in Go
BSD 3-Clause "New" or "Revised" License
4.62k stars 428 forks source link

race between `DB.deleters.Add` and `DB.deleters.Wait`. #1266

Open bananabrick opened 2 years ago

bananabrick commented 2 years ago

If the WaitGroup counter is at 0, then WaitGroup.Add must be called before WaitGroup.Wait is called, see: https://pkg.go.dev/sync#WaitGroup.Add.

In our code, we call deleters.Add, here: https://github.com/cockroachdb/pebble/blob/master/compaction.go#L2821.

And we wait, here: https://github.com/cockroachdb/pebble/blob/master/db.go#L1016.

But, DB.Close could be called before Add is ever called. Before we call deleters.Wait in DB.Close, we do call DB.deleteObsoleteFiles, here: https://github.com/cockroachdb/pebble/blob/master/db.go#L1012, which may call deleters.Add, here: https://github.com/cockroachdb/pebble/blob/master/compaction.go#L2821, but only if there exist files to delete, https://github.com/cockroachdb/pebble/blob/master/compaction.go#L2820.

Seems pretty minor and maybe I'm missing an invariant which prevents this.

Jira issue: PEBBLE-204

github-actions[bot] commented 3 months ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to Pebble!