From d5a6d348253c2b09ff7a87c65f8e1648fb83d8ad Mon Sep 17 00:00:00 2001 From: ihsandemir Date: Fri, 8 Jan 2021 08:49:38 +0300 Subject: [PATCH 1/2] Removed destruction of the thread_pool on shutdown and left it to actual client destruction. Failing to do this causes illegal memory access during destruction of stands in the listener_service_impl. Removed the work around of bug https://github.com/chriskohlhoff/asio/issues/431. Added correct handling of the invocation retry timer. If it is not assigned to a timer variable in the invocation then it will actually be destroyed immediately, which will cause the timer cancelled immediately on destruction but this is not what we want. --- .../hazelcast/client/spi/impl/ClientInvocation.h | 3 +++ hazelcast/src/hazelcast/client/network.cpp | 8 ++++---- hazelcast/src/hazelcast/client/spi.cpp | 2 +- hazelcast/src/hazelcast/util/util.cpp | 10 ---------- 4 files changed, 8 insertions(+), 15 deletions(-) diff --git a/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h b/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h index c866393c57..793bb0b3fa 100644 --- a/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h +++ b/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h @@ -24,6 +24,7 @@ #include #include #include +#include #include "hazelcast/util/Sync.h" #include "hazelcast/client/exception/protocol_exceptions.h" @@ -179,6 +180,8 @@ namespace hazelcast { */ std::chrono::steady_clock::time_point pending_response_received_time_; + std::shared_ptr retry_timer_; + ClientInvocation(spi::ClientContext &client_context, std::shared_ptr &&message, const std::string &name, int partition = UNASSIGNED_PARTITION, diff --git a/hazelcast/src/hazelcast/client/network.cpp b/hazelcast/src/hazelcast/client/network.cpp index df22a2c656..92a344280d 100644 --- a/hazelcast/src/hazelcast/client/network.cpp +++ b/hazelcast/src/hazelcast/client/network.cpp @@ -386,11 +386,11 @@ namespace hazelcast { submit_connect_to_cluster_task(); } - } catch (exception::iexception &e) { + } catch (std::exception &e) { HZ_LOG(logger_, warning, - boost::str(boost::format("Could not connect to any cluster, " - "shutting down the client: %1%") - % e) + boost::str(boost::format("Could not connect to any cluster, " + "shutting down the client: %1%") + % e.what()) ); shutdown_with_external_thread(client_.get_hazelcast_client_implementation()); diff --git a/hazelcast/src/hazelcast/client/spi.cpp b/hazelcast/src/hazelcast/client/spi.cpp index d6fb9a6e12..69996378ed 100644 --- a/hazelcast/src/hazelcast/client/spi.cpp +++ b/hazelcast/src/hazelcast/client/spi.cpp @@ -1458,7 +1458,7 @@ namespace hazelcast { // progressive retry delay int64_t delayMillis = util::min(static_cast(1) << (invoke_count_ - MAX_FAST_INVOCATION_COUNT), std::chrono::duration_cast(retry_pause_).count()); - execution_service_->schedule(command, std::chrono::milliseconds(delayMillis)); + retry_timer_ = execution_service_->schedule(command, std::chrono::milliseconds(delayMillis)); } } diff --git a/hazelcast/src/hazelcast/util/util.cpp b/hazelcast/src/hazelcast/util/util.cpp index d8878cbfec..afdcdea492 100644 --- a/hazelcast/src/hazelcast/util/util.cpp +++ b/hazelcast/src/hazelcast/util/util.cpp @@ -1023,16 +1023,6 @@ namespace hazelcast { void hz_thread_pool::shutdown_gracefully() { pool_->join(); - pool_->stop(); - -#if defined(WIN32) || defined(_WIN32) || defined(WIN64) || defined(_WIN64) - // needed due to bug https://github.com/chriskohlhoff/asio/issues/431 - boost::asio::use_service(*pool_).stop(); - // We need the following line so that the windows threads can be terminated. Otherwise, - // the threads stay dangling and cause resource leakage. Normally, the pool_ is destructed on client - // destruction but somehow it needs to be triggered at this point. - pool_.reset(); -#endif } boost::asio::thread_pool::executor_type hz_thread_pool::get_executor() const { From a6d90fc2f4a2e7b56b6e9770042cdfa03228321f Mon Sep 17 00:00:00 2001 From: ihsandemir Date: Fri, 8 Jan 2021 13:07:30 +0300 Subject: [PATCH 2/2] Double check client shutdown on connection opening. --- hazelcast/src/hazelcast/client/network.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hazelcast/src/hazelcast/client/network.cpp b/hazelcast/src/hazelcast/client/network.cpp index 92a344280d..1ea95fe24f 100644 --- a/hazelcast/src/hazelcast/client/network.cpp +++ b/hazelcast/src/hazelcast/client/network.cpp @@ -645,6 +645,12 @@ namespace hazelcast { on_connection_close(connection); return nullptr; } + + // If the client is shutdown in parallel, we need to close this new connection. + if (!client_.get_lifecycle_service().is_running()) { + connection->close("Client is shutdown"); + } + return connection; }