From 59bada828911b9d30bec9fb2fb03a5b78efac990 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Wed, 13 May 2026 11:21:34 -0400 Subject: [PATCH 1/2] Catch exceptions in EventHubs client destructors Wrap the Close() calls in ProducerClient, ConsumerClient, and PartitionClient destructors with try/catch so a throw from Close() no longer terminates the program via std::terminate. Caught exceptions are logged at Warning level. Each Log::Stream call is itself wrapped so a logger allocation failure or a throwing user listener cannot escape the noexcept destructor. Fixes #6957 --- .../azure-messaging-eventhubs/CHANGELOG.md | 2 + .../messaging/eventhubs/consumer_client.hpp | 2 +- .../messaging/eventhubs/partition_client.hpp | 2 +- .../messaging/eventhubs/producer_client.hpp | 2 +- .../src/consumer_client.cpp | 37 +++++++++++++++-- .../src/partition_client.cpp | 40 +++++++++++++++++-- .../src/producer_client.cpp | 30 ++++++++++++++ 7 files changed, 105 insertions(+), 10 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md index c721bf7e64..19e4ade1c8 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md +++ b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md @@ -11,6 +11,8 @@ ### Bugs Fixed +- [[#6957]](https://github.com/Azure/azure-sdk-for-cpp/issues/6957) Catch and log exceptions thrown from `Close()` inside the `ProducerClient`, `ConsumerClient`, and `PartitionClient` destructors so cleanup failures no longer terminate the program via `std::terminate`. + ### Other Changes ## 1.0.0-beta.10 (2024-11-01) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp index 98daf20a93..e3e63674a9 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp @@ -106,7 +106,7 @@ namespace Azure { namespace Messaging { namespace EventHubs { /** Move a consumer client */ ConsumerClient& operator=(ConsumerClient&& other) = delete; - ~ConsumerClient(); + ~ConsumerClient() noexcept; /** @brief Getter for event hub name * diff --git a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp index 501bb16815..73f8d463a9 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/partition_client.hpp @@ -70,7 +70,7 @@ namespace Azure { namespace Messaging { namespace EventHubs { /** Destroy this partition client. */ - virtual ~PartitionClient(); + virtual ~PartitionClient() noexcept; /** Receive events from the partition. * diff --git a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp index b10d935999..8d77806d39 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp @@ -98,7 +98,7 @@ namespace Azure { namespace Messaging { namespace EventHubs { /** Default Constructor for a ProducerClient */ ProducerClient() = default; - ~ProducerClient() { Close(); } + ~ProducerClient() noexcept; /** @brief Close all the connections and sessions. * diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp b/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp index 02fed577c1..3c43c798c3 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp @@ -28,11 +28,42 @@ namespace Azure { namespace Messaging { namespace EventHubs { + _detail::EventHubsConsumerGroupsPath + m_consumerGroup; } - ConsumerClient::~ConsumerClient() + ConsumerClient::~ConsumerClient() noexcept { - Log::Stream(Logger::Level::Informational) << "Destroy consumer client."; + try + { + Log::Stream(Logger::Level::Informational) << "Destroy consumer client."; + } + catch (...) + { + } - Close({}); + try + { + Close({}); + } + catch (std::exception const& e) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~ConsumerClient(): exception thrown during Close(): " << e.what(); + } + catch (...) + { + } + } + catch (...) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~ConsumerClient(): unknown exception thrown during Close()."; + } + catch (...) + { + } + } } void ConsumerClient::Close(Azure::Core::Context const& context) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp b/sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp index af734ecbdc..4466ec4680 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/partition_client.cpp @@ -208,11 +208,43 @@ namespace Azure { namespace Messaging { namespace EventHubs { { } - PartitionClient::~PartitionClient() + PartitionClient::~PartitionClient() noexcept { - Log::Stream(Logger::Level::Verbose) << "~PartitionClient() " - << "Close Receiver."; - m_receiver.Close(); + try + { + Log::Stream(Logger::Level::Verbose) << "~PartitionClient() " + << "Close Receiver."; + } + catch (...) + { + } + + try + { + m_receiver.Close(); + } + catch (std::exception const& e) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~PartitionClient(): exception thrown during m_receiver.Close(): " << e.what(); + } + catch (...) + { + } + } + catch (...) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~PartitionClient(): unknown exception thrown during m_receiver.Close()."; + } + catch (...) + { + } + } } /** Receive events from the partition. diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp b/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp index e14c63088c..ff409ebac3 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp @@ -33,6 +33,36 @@ namespace Azure { namespace Messaging { namespace EventHubs { { } + ProducerClient::~ProducerClient() noexcept + { + try + { + Close(); + } + catch (std::exception const& e) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~ProducerClient(): exception thrown during Close(): " << e.what(); + } + catch (...) + { + } + } + catch (...) + { + try + { + Log::Stream(Logger::Level::Warning) + << "~ProducerClient(): unknown exception thrown during Close()."; + } + catch (...) + { + } + } + } + void ProducerClient::Close(Azure::Core::Context const& context) { Log::Stream(Logger::Level::Verbose) << "Close producer client."; From 1bdd6317b46e4d9b8c72f276ad776eed12d17487 Mon Sep 17 00:00:00 2001 From: Johnathan W Date: Wed, 13 May 2026 11:58:00 -0400 Subject: [PATCH 2/2] Make EventHubs Close() best-effort Refactor ProducerClient::Close and ConsumerClient::Close so a throw from one teardown step no longer skips the rest. Each step (properties client, sender/receiver loop, session loop, connection loop) runs in its own try/catch. The first exception is captured and rethrown to the caller after every step has been attempted; subsequent throws are logged and swallowed. Also fix two copy-paste log message typos in ConsumerClient::Close ("Close producer client." and "Closing message senders.") and replace the equivalent-but-cryptic `while(!empty) erase(begin)` drain pattern with `clear()` to match ProducerClient. --- .../azure-messaging-eventhubs/CHANGELOG.md | 1 + .../messaging/eventhubs/consumer_client.hpp | 4 ++ .../messaging/eventhubs/producer_client.hpp | 4 ++ .../src/consumer_client.cpp | 70 +++++++++++++++---- .../src/producer_client.cpp | 56 +++++++++++++-- 5 files changed, 116 insertions(+), 19 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md index 19e4ade1c8..41f0a841d5 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md +++ b/sdk/eventhubs/azure-messaging-eventhubs/CHANGELOG.md @@ -12,6 +12,7 @@ ### Bugs Fixed - [[#6957]](https://github.com/Azure/azure-sdk-for-cpp/issues/6957) Catch and log exceptions thrown from `Close()` inside the `ProducerClient`, `ConsumerClient`, and `PartitionClient` destructors so cleanup failures no longer terminate the program via `std::terminate`. +- `ProducerClient::Close` and `ConsumerClient::Close` now perform best-effort teardown: a throw from one step no longer skips remaining cleanup. The first exception encountered is rethrown to the caller after every step has been attempted. ### Other Changes diff --git a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp index e3e63674a9..61721edf90 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/consumer_client.hpp @@ -158,6 +158,10 @@ namespace Azure { namespace Messaging { namespace EventHubs { * partition clients. * * @param context The context for the operation can be used for request cancellation. + * + * @remark Performs best-effort teardown: if one step throws, the remaining + * steps are still attempted. The first exception encountered is rethrown + * to the caller after all steps have run. */ void Close(Azure::Core::Context const& context); diff --git a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp index 8d77806d39..3f5f249e99 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/inc/azure/messaging/eventhubs/producer_client.hpp @@ -103,6 +103,10 @@ namespace Azure { namespace Messaging { namespace EventHubs { /** @brief Close all the connections and sessions. * * @param context Context for the operation can be used for request cancellation. + * + * @remark Performs best-effort teardown: if one step throws, the remaining + * steps are still attempted. The first exception encountered is rethrown + * to the caller after all steps have run. */ void Close(Azure::Core::Context const& context = {}); diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp b/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp index 3c43c798c3..5a1ae8d9c0 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/consumer_client.cpp @@ -68,45 +68,85 @@ namespace Azure { namespace Messaging { namespace EventHubs { void ConsumerClient::Close(Azure::Core::Context const& context) { - Log::Stream(Logger::Level::Verbose) << "Close producer client."; + Log::Stream(Logger::Level::Verbose) << "Close consumer client."; + + // Best-effort teardown: a throw from one step must not skip the rest. The + // first exception encountered is captured and rethrown after every step + // has been attempted. + std::exception_ptr firstException; + auto attemptStep + = [&firstException](auto&& step, char const* description) noexcept { + try + { + step(); + } + catch (std::exception const& e) + { + if (!firstException) + { + firstException = std::current_exception(); + } + try + { + Log::Stream(Logger::Level::Warning) + << "ConsumerClient::Close: " << description << " failed: " << e.what(); + } + catch (...) + { + } + } + catch (...) + { + if (!firstException) + { + firstException = std::current_exception(); + } + try + { + Log::Stream(Logger::Level::Warning) << "ConsumerClient::Close: " << description + << " failed with unknown exception."; + } + catch (...) + { + } + } + }; + { std::unique_lock lock(m_propertiesClientLock); if (m_propertiesClient) { - m_propertiesClient->Close(context); + attemptStep([&] { m_propertiesClient->Close(context); }, "properties client"); m_propertiesClient.reset(); } } - Log::Stream(Logger::Level::Verbose) << "Closing message senders."; - // Tear down the sessions and then the connections, in that order. + Log::Stream(Logger::Level::Verbose) << "Closing message receivers."; for (auto& receiver : m_receivers) { - receiver.second.Close(context); + attemptStep([&] { receiver.second.Close(context); }, "message receiver"); } #if ENABLE_RUST_AMQP Log::Stream(Logger::Level::Verbose) << "Closing sessions."; for (auto& session : m_sessions) { - session.second.End(context); + attemptStep([&] { session.second.End(context); }, "session"); } Log::Stream(Logger::Level::Verbose) << "Closing connections."; for (auto& connection : m_connections) { - connection.second.Close(context); + attemptStep([&] { connection.second.Close(context); }, "connection"); } #endif - while (!m_sessions.empty()) - { - m_sessions.erase(m_sessions.begin()); - } + m_sessions.clear(); + m_connections.clear(); + m_receivers.clear(); - while (!m_connections.empty()) + if (firstException) { - m_connections.erase(m_connections.begin()); - }; - m_receivers.clear(); + std::rethrow_exception(firstException); + } } Azure::Core::Amqp::_internal::Connection ConsumerClient::CreateConnection( diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp b/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp index ff409ebac3..06f63dc702 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/producer_client.cpp @@ -66,18 +66,61 @@ namespace Azure { namespace Messaging { namespace EventHubs { void ProducerClient::Close(Azure::Core::Context const& context) { Log::Stream(Logger::Level::Verbose) << "Close producer client."; + + // Best-effort teardown: a throw from one step must not skip the rest. The + // first exception encountered is captured and rethrown after every step + // has been attempted. + std::exception_ptr firstException; + auto attemptStep + = [&firstException](auto&& step, char const* description) noexcept { + try + { + step(); + } + catch (std::exception const& e) + { + if (!firstException) + { + firstException = std::current_exception(); + } + try + { + Log::Stream(Logger::Level::Warning) + << "ProducerClient::Close: " << description << " failed: " << e.what(); + } + catch (...) + { + } + } + catch (...) + { + if (!firstException) + { + firstException = std::current_exception(); + } + try + { + Log::Stream(Logger::Level::Warning) << "ProducerClient::Close: " << description + << " failed with unknown exception."; + } + catch (...) + { + } + } + }; + { std::unique_lock lock(m_propertiesClientLock); if (m_propertiesClient) { - m_propertiesClient->Close(context); + attemptStep([&] { m_propertiesClient->Close(context); }, "properties client"); m_propertiesClient.reset(); } } Log::Stream(Logger::Level::Verbose) << "Closing message senders."; for (auto& sender : m_senders) { - sender.second.Close(context); + attemptStep([&] { sender.second.Close(context); }, "message sender"); } m_senders.clear(); @@ -85,17 +128,22 @@ namespace Azure { namespace Messaging { namespace EventHubs { Log::Stream(Logger::Level::Verbose) << "Closing sessions."; for (auto& session : m_sessions) { - session.second.End(context); + attemptStep([&] { session.second.End(context); }, "session"); } Log::Stream(Logger::Level::Verbose) << "Closing connections."; for (auto& connection : m_connections) { - connection.second.Close(context); + attemptStep([&] { connection.second.Close(context); }, "connection"); } #endif // Remove all the sessions and connections after they've been closed. m_sessions.clear(); m_connections.clear(); + + if (firstException) + { + std::rethrow_exception(firstException); + } } EventDataBatch ProducerClient::CreateBatch(