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

Open Beginner friendly
#1,239 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.