apache / apache/rocketmq-client-go
[Bug] Orderly consumption per-queue lock is ineffective: QueueLock.fetchLock value receiver copies the sync.Map and returns a new mutex every call
- Dominant language
- Go
- Stars
- 1.4k
- Forks
- 445
- PR merge metrics
- No merged PRs in 30d
Description
### Describe the Bug
`QueueLock.fetchLock` in `consumer/lock.go` uses a **value receiver** on a struct that contains a `sync.Map`:
```go
type QueueLock struct { lockTable sync.Map }
func (ql QueueLock) fetchLock(queue primitive.MessageQueue) sync.Locker {
v, _ := ql.lockTable.LoadOrStore(queue, new(sync.Mutex))
return v.(*sync.Mutex)
}
```
Every call copies the whole struct (flagged by `go vet` copylocks), so `LoadOrStore` writes into a discarded copy and the original map stays empty forever — **each call returns a brand-new `*sync.Mutex`**.
The only caller is `consumeMessageOrderly` (`consumer/push_consumer.go`): `lock := pc.queueLock.fetchLock(*mq); lock.Lock()`. Since every goroutine gets its own mutex, the per-queue mutual exclusion is completely ineffective: multiple goroutines can process the same `MessageQueue` concurrently, breaking the FIFO guarantee that orderly consumption exists to provide.
### Steps to Reproduce
Two deterministic tests (included in the incoming PR):
1. Identity: call `fetchLock` twice for the same queue — unfixed returns two different mutexes.
2. Mutual exclusion: 20 goroutines fetch the lock for the same queue and record max concurrency — unfixed observes `maxConcurrent=20` (zero serialization); fixed observes `maxConcurrent=1`.
`go vet ./consumer/` also reports the copylocks diagnostic on this method.
### What Did You Expect to See?
Same queue → same lock; orderly consumption strictly serialized per queue.
### What Did You See Instead?
A fresh lock per call; per-queue ordering not enforced.
### Additional Context
Fix incoming: change the receiver to `*QueueLock` (one character). `go vet` copylocks disappears; both tests flip from FAIL to PASS; the consumer suite passes with no regressions.
Contributor guide
Research direction
Start in consumer/lock.go by inspecting QueueLock.fetchLock and then trace its only caller in consumer/push_consumer.go. Run go vet ./consumer/ and the consumer test suite; done means the copylocks diagnostic is gone, the same queue reuses one lock, and orderly consumption remains serialized.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 95/100