apache / apache/pulsar-client-go

[Bug] Concurrent transactional ACKs can crash in registerAckTopic

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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.