apache / apache/pulsar-client-go
[Bug] Concurrent transactional ACKs can crash in registerAckTopic
- Dominant language
- Go
- Stars
- 745
- Forks
- 389
- Avg merge
- 3d 20h
- Merged PRs (30d)
- 3
Description
#### Expected behavior
Concurrent transactional acknowledgments for different partitions using the same transaction should safely register each `(topic, subscription)`.
#### Actual behavior
In upstream master, `registerAckTopic` reads the ordinary Go map `registerAckSubscriptions` without holding `txn.mu`. It subsequently acquires the mutex and writes the same map after a successful registration. Concurrent calls can therefore cause a data race and the Go runtime failure `concurrent map read and map write`.
This report is based on the public upstream implementation:
- [Map declaration](https://github.com/apache/pulsar-client-go/blob/61d7a95e66cddf329adc6d925a8bd60ccd26eda5/pulsar/transaction_impl.go#L37-L43)
- [Unprotected lookup and protected write in registerAckTopic](https://github.com/apache/pulsar-client-go/blob/61d7a95e66cddf329adc6d925a8bd60ccd26eda5/pulsar/transaction_impl.go#L196-L216)
A mutex around the write alone does not protect a reader that does not acquire that mutex. Accesses to different keys still share the same map.
#### Steps to reproduce
The conflicting interleaving permitted by the current code is:
1. Two partition-consumer event loops acknowledge messages using the same transaction.
2. Call A misses its initial lookup, acquires `txn.mu`, and registers its subscription with the coordinator.
3. While call A writes `registerAckSubscriptions`, call B performs the initial map lookup without acquiring `txn.mu`.
A workload with concurrent `AckWithTxn` calls across partitions and fresh transactions can exercise this path. The failure is timing-dependent; this is an explanation of the source-level interleaving, not a claim of a deterministic standalone reproducer.
The minimal fix is to remove the initial unlocked lookup and perform the lookup, first registration, and map update under the existing mutex. Populate the map only after successful registration to preserve error and retry behavior.
#### System configuration
- **Client source**: upstream master `61d7a95e66cddf329adc6d925a8bd60ccd26eda5`.
- **Pulsar version**: this is a client-side map synchronization defect; no broker-version-specific behavior is required by the conflicting interleaving.
Contributor guide
Research direction
Start in pulsar/transaction_impl.go at registerAckTopic and inspect the registerAckSubscriptions declaration and the lookup/write sequence. Exercise concurrent AckWithTxn calls across partitions with fresh transactions under the Go race detector. Done means lookup, registration, and map updates are synchronized, successful registrations are stored, and retry behavior remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100