Azure / Azure/azure-sdk-for-cpp
[EventHubs] PartitionClient::Close does not stop a later receiver rebuild
- Dominant language
- C++
- Stars
- 205
- Forks
- 172
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 37
Description
## Summary
`PartitionClient::Close` records no state. A `ReceiveEvents` call that runs after `Close` reads the local close as a link fault, attaches a new receiver on the still open session, and returns events. Before #7328 that call threw.
## Motivation
`Close` is one statement, `m_receiver.Close(context)` (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:228`). It sets no flag, and the object keeps its session, its partition address, and its link name.
`ReceiveEvents` tests no closed state. Its loop reads from the receiver and it sends any fault to the `recover` lambda (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:392-430`). A read on a closed receiver reports a fault with an empty condition, and `ShouldRebuildReceiver` permits a new attach for an empty condition (`sdk/eventhubs/azure-messaging-eventhubs/src/private/eventhubs_utilities.hpp:98-101`).
`RebuildReceiver` then closes the receiver again, builds a new receiver on `m_session`, and opens it (`sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp:230-262`). `ConsumerClient` owns the session and the connection, and `Close` does not touch either one, so the new attach succeeds and the call returns events.
The previous contract was clear: a receive after a close threw. The rebuild path removed that contract without a decision to remove it.
## Proposal
Add an `std::atomic m_closed` member to `PartitionClient`, with the value false at construction. `Close` sets it to true before it closes the receiver.
`ReceiveEvents` tests the flag when it starts, and it throws an `EventHubsException` with `IsTransient` false when the flag is true. The `recover` lambda tests the same flag before each rebuild attempt, so a close that lands during a backoff stops the rebuild instead of racing it.
Add a test that calls `Close` and then calls `ReceiveEvents`, and that asserts the throw. The mock server cannot serve a reattach today, so this test must use a client that is already closed, and not a fault that arrives from the wire.
This defect came in with #7328, which is the link reattach part of #7254.
Contributor guide
Research direction
Start in sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp, reading Close, ReceiveEvents, and RebuildReceiver, then inspect ShouldRebuildReceiver in sdk/eventhubs/azure-messaging-eventhubs/src/private/eventhubs_utilities.hpp. Run the existing Event Hubs tests and add coverage for receiving after Close; done means it throws a non-transient EventHubsException and does not rebuild the receiver after a concurrent close.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100