cockroachdb / cockroachdb/cockroach

kvserver/concurrency: tryClearLocksGE reads lock holder state without kl.mu

Open Beginner friendly
#172,429 0 comments 0 reactions 0 assignees View on GitHub
A-kv-transactions branch-master C-bug O-agent P-3 T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

This is a latent hazard: race-free today, but only by a global argument that nothing enforces.

`tryClearLocksGE` reads `l.holders` and `unreplicatedInfo` without acquiring `kl.mu` ([lock_table.go#L4927-L4948](https://github.com/cockroachdb/cockroach/blob/a7e117882b4356096b3a8ea26046baa7e5d53fce/pkg/kv/kvserver/concurrency/lock_table.go#L4927-L4948)). This is currently safe because it holds `treeMu.mu` exclusively and every existing `holders` mutator happens to hold at least a tree read lock. But the `keyLocks` struct comment says `kl.mu` protects this state, so a future mutation path that (correctly, per the comment) holds only `kl.mu` would create a data race with no locally visible cause.

`ExportUnreplicatedLocks` takes `l.mu` for the identical read.

**Suggested fix**

Take `l.mu` in `tryClearLocksGE` as `ExportUnreplicatedLocks` does. The lock is uncontended in this context, so this is purely defensive cost.

**Additional context**

Found during an agent-assisted correctness audit of `pkg/kv/kvserver/concurrency`. Code links are pinned to master @ a7e1178.

Jira issue: CRDB-65665

Contributor guide

Open the contributing guide

Research direction

Start in pkg/kv/kvserver/concurrency/lock_table.go at tryClearLocksGE and compare its reads with ExportUnreplicatedLocks. Verify how kl.mu protects holders and unreplicatedInfo, then confirm the change preserves the existing tree-lock context and removes the latent unsynchronized read.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.