Summary
PartitionClient documents no thread safety contract. A Close that runs at the same time as ReceiveEvents on one instance is a data race on m_receiver, and it can leave an open receiver attached after Close returned.
Motivation
m_receiver is a value wrapper that holds a non-atomic std::shared_ptr (sdk/core/azure-core-amqp/inc/azure/core/amqp/internal/message_receiver.hpp:191). Close reads that pointer at sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:272 and RebuildReceiver writes it at :303, with no synchronization, so concurrent calls are undefined behavior. There is also a window between the closed test at :412 and the assignment at :303: a Close that lands inside it lets the rebuild attach a new receiver after Close returned. ReceiveEvents also mutates m_lastReceivedOffset (:356) and m_pendingError (:396, :480) with no lock. The exposure predates PR #7337; on feat/link-reattach the recover loop rebuilds with no closed test at all. The Azure SDK for C++ guidelines state "DO be thread-safe. All public members of the client type must be safe to call from multiple threads concurrently", and this class does not meet that today. The class doc at sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp:55-59, the ConsumerClient doc, and the Event Hubs README say nothing about threads, so a caller has no stated contract to follow.
Proposal
Decide the contract first, then make the code match it. Two options are open. The first option documents PartitionClient as safe for one thread at a time and states that a caller must not call Close while ReceiveEvents runs. The second option meets the guideline: move the closed state into the receiver implementation as a latch, or swap the implementation pointer atomically, so Close and a rebuild cannot race. A mutex around the closed test and the rebuild does not work, because Close would block uninterruptibly while RebuildReceiver holds the lock across a network attach, and protecting the receiver reads at :492 and :511 would hold the lock across WaitForIncomingMessage for the full read deadline.
Found while reviewing #7337, which fixes the sequential case in #7334.
Summary
PartitionClientdocuments no thread safety contract. AClosethat runs at the same time asReceiveEventson one instance is a data race onm_receiver, and it can leave an open receiver attached afterClosereturned.Motivation
m_receiveris a value wrapper that holds a non-atomicstd::shared_ptr(sdk/core/azure-core-amqp/inc/azure/core/amqp/internal/message_receiver.hpp:191).Closereads that pointer atsdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:272andRebuildReceiverwrites it at:303, with no synchronization, so concurrent calls are undefined behavior. There is also a window between the closed test at:412and the assignment at:303: aClosethat lands inside it lets the rebuild attach a new receiver afterClosereturned.ReceiveEventsalso mutatesm_lastReceivedOffset(:356) andm_pendingError(:396,:480) with no lock. The exposure predates PR #7337; onfeat/link-reattachthe recover loop rebuilds with no closed test at all. The Azure SDK for C++ guidelines state "DO be thread-safe. All public members of the client type must be safe to call from multiple threads concurrently", and this class does not meet that today. The class doc atsdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp:55-59, theConsumerClientdoc, and the Event Hubs README say nothing about threads, so a caller has no stated contract to follow.Proposal
Decide the contract first, then make the code match it. Two options are open. The first option documents
PartitionClientas safe for one thread at a time and states that a caller must not callClosewhileReceiveEventsruns. The second option meets the guideline: move the closed state into the receiver implementation as a latch, or swap the implementation pointer atomically, soCloseand a rebuild cannot race. A mutex around the closed test and the rebuild does not work, becauseClosewould block uninterruptibly whileRebuildReceiverholds the lock across a network attach, and protecting the receiver reads at:492and:511would hold the lock acrossWaitForIncomingMessagefor the full read deadline.Found while reviewing #7337, which fixes the sequential case in #7334.