fix(core-amqp): leave an object closed when its close fails

[Johnathan W (j7nw4r)]

Summary

ManagementClientImpl::Close, MessageSenderImpl::Close, and MessageReceiverImpl::Close clear the open flag only on the last line. A close that throws skips that line and leaves the object open. The destructor then calls AzureNoReturnPath, which stops the process in a Debug build and in a Release build. The caller cannot catch this.

Fixes #7323.

Motivation

MessageSenderImpl::Close waits on its close queue with the context of the caller. A cancelled context, or a peer that sends no detach, makes WaitForResult return nothing. The close then throws OperationCancelledException. An Event Hubs caller that puts a deadline on a properties call therefore stops the process.

The uAMQP sender and receiver also run their teardown after that wait, so a throw skips it. The teardown calls EnableAsyncOperation(false). Without that call the poll count stays raised, and ~ConnectionImpl aborts with "Connection is being destroyed while polling". A fix that only clears the flag moves the abort. ManagementClientImpl::Close skips its receiver close when the sender close throws, and the open receiver aborts in its destructor.

Pull request #7308 rebases on this change. Issue #7327 records the one open item in this area.

Changes

• MessageSenderImpl and MessageReceiverImpl (uAMQP) move the teardown into a private CompleteClose method. Close calls it on the normal path and in a catch block that rethrows. CompleteClose releases the link, calls EnableAsyncOperation(false), and clears the open flag.
• ManagementClientImpl::Close (uAMQP) runs both closes, keeps the first exception, clears its own flag, and rethrows.
• ManagementClientImpl::Close and MessageSenderImpl::Close (Rust) clear the flag before the detach call. This matches the Rust MessageReceiverImpl::Close, which already did this.
• No close gets its own deadline. A ten second deadline stopped the Event Hubs concurrency test on each run. That test passes in 63 seconds when the close keeps the context of the caller.
• The changelog gets an entry under ### Bugs Fixed for 1.0.0-beta.13.

Test plan

Three tests open an object, close it with a context that is already cancelled, and then destroy it.

• TestMessageSendReceive.SenderCloseWithCancelledContext and TestMessageSendReceive.ReceiverCloseWithCancelledContext in sdk/core/azure-core-amqp/test/ut/message_sender_receiver.cpp. Both register a mock service endpoint. Both assert EXPECT_THROW(..., Azure::Core::OperationCancelledException) on uAMQP and EXPECT_NO_THROW on Rust.
• The receiver test also waits for the Open state through a MessageReceiverEvents handler. MessageReceiverImpl::Open returns before the link attaches, so an immediate close leaves shouldWaitForClose false and never reads the context.
• TestManagement.ManagementCloseWithCancelledContext in sdk/core/azure-core-amqp/test/ut/management_tests.cpp asserts the same exception. Its body sits behind #if !defined(USE_NATIVE_BROKER), so it is empty on a Rust build.

All three sit inside the #if !defined(AZ_PLATFORM_MAC) block that holds the tests around them.

The pipeline builds the Rust stack alone, because sdk/core/azure-core-amqp/CMakeLists.txt forces USE_RUST_AMQP on unless DISABLE_RUST_IN_BUILD is set. A cancelled context cannot make a Rust detach fail. amqpmessagesender_detach_and_release in rust_wrapper/src/amqp/message_sender.rs uses the call context only for the runtime handle and the error slot, then calls block_on(sender.inner.detach()).

Validation

A Linux container ran the tests on the branch, then ran them again with the production code of origin/main restored.

Test	With the change	With the production code of main
TestMessageSendReceive.SenderCloseWithCancelledContext	exit 0, [ OK ] ... (510 ms)	exit 134, AzureNoReturnPath (msg="MessageSenderImpl is being destroyed while open.") at message_sender.cpp:125
TestMessageSendReceive.ReceiverCloseWithCancelledContext	exit 0, [ OK ] ... (516 ms)	exit 134, AzureNoReturnPath (msg="MessageReceiverImpl is being destroyed while open.") at message_receiver.cpp:257
TestManagement.ManagementCloseWithCancelledContext	exit 0, [ OK ] ... (720 ms)	exit 134, management.cpp:37: ~ManagementClientImpl(): Assertion ((void)("Management being destroyed while open."), (!m_isOpen)) failed

• Each abort site above comes from a gdb backtrace of the aborting run, so the frame and the message text are exact.
• The full uAMQP suite passes with the change: [ PASSED ] 167 tests, exit 0. With the production code of main it reaches 120 tests, then aborts at TestManagement.ManagementCloseWithCancelledContext with exit 134.
• The build: ubuntu:22.04, GCC 11.4.0, CMake 4.3.4 from the Kitware release (vcpkg needs string(JSON ... STRING_ENCODE) and Ubuntu 22.04 ships CMake 3.22), and vcpkg at the builtin-baseline of vcpkg.json. Then cmake -S . -B build-uamqp -G Ninja -DCMAKE_TOOLCHAIN_FILE=<vcpkg>/scripts/buildsystems/vcpkg.cmake -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTING=ON -DDISABLE_RUST_IN_BUILD=ON and cmake --build build-uamqp --target azure-core-amqp-tests. The build needs no WARNINGS_AS_ERRORS=OFF.
• The container is linux/arm64 and the pipeline uses x86_64. The defect sits in C++ control flow and not in code generation, but this measurement does not cover x86_64.
• The pipeline covers the Rust stack, which this measurement did not build.

[azure-pipelines]

Azure Pipelines:
Successfully started running 2 pipeline(s).
8 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Prevents failed AMQP closes from leaving objects open and aborting during destruction.

Changes:

• Ensures uAMQP teardown runs even when close throws.
• Marks Rust AMQP objects closed before detach.
• Adds regression tests and changelog documentation.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
sdk/core/azure-core-amqp/test/ut/message_sender_receiver.cpp	Adds cancelled-context close tests.
sdk/core/azure-core-amqp/test/ut/management_tests.cpp	Adds management close regression test.
sdk/core/azure-core-amqp/src/impl/uamqp/amqp/private/message_sender_impl.hpp	Declares sender teardown helper.
sdk/core/azure-core-amqp/src/impl/uamqp/amqp/private/message_receiver_impl.hpp	Declares receiver teardown helper.
sdk/core/azure-core-amqp/src/impl/uamqp/amqp/message_sender.cpp	Guarantees sender teardown after failures.
sdk/core/azure-core-amqp/src/impl/uamqp/amqp/message_receiver.cpp	Guarantees receiver teardown after failures.
sdk/core/azure-core-amqp/src/impl/uamqp/amqp/management.cpp	Closes both links and rethrows the first failure.
sdk/core/azure-core-amqp/src/impl/rust_amqp/amqp/message_sender.cpp	Clears sender state before detach.
sdk/core/azure-core-amqp/src/impl/rust_amqp/amqp/management.cpp	Clears management state before detach.
sdk/core/azure-core-amqp/CHANGELOG.md	Documents the bug fix.

Suppressed comments (1)

sdk/core/azure-core-amqp/test/ut/message_sender_receiver.cpp:536

• This test treats both throwing and returning normally as success, so it does not verify that the original close exception still reaches the caller. Assert the transport-specific outcome to cover the new rethrow behavior rather than relying only on destructor survival.

        sender.Close(cancelledContext);

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Johnathan W (j7nw4r)]

/azp run cpp - storage - ci

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[Johnathan W (j7nw4r)]

Closing this one. The change already reached main through #7308, which merged as c6c3f94. That branch sat on top of this commit, so the merge carried this fix in as its first commit.

Everything here is in main today: CompleteClose in the uAMQP sender and receiver, the first-exception handling in the uAMQP management client, the flag-before-detach on the Rust side, all three EXPECT_THROW(..., OperationCancelledException) tests, and the changelog bullet. #7323 is closed. The only diff left against main is a duplicate of that same changelog bullet, which is what marks this pull request as conflicting.