Azure / Azure/azure-sdk-for-cpp

[EventHubs] PartitionClient::Close does not stop a later receiver rebuild

Open
#7,334 1 comment 0 reactions 0 assignees View on GitHub
Client Event Hubs needs-team-attention Service Attention
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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.