From 3e1f7dd51bcc3b47be64a7303c0a897ba0ef50d2 Mon Sep 17 00:00:00 2001 From: ihsan demir Date: Mon, 1 Jun 2026 14:00:21 +0000 Subject: [PATCH 1/3] Fix rfc2818_verification build failure on Windows with Boost 1.83.0 and OpenSSL 3.x [HZ-5424] (#1464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1463 ## Summary - The Boost 1.83.0 + OpenSSL 3.x matrix entry of the `nightly-Windows` workflow fails to compile `rfc2818_verification.ipp`: `error C2027: use of undefined type 'asn1_string_st'` (followed by a cascading `C2660` on `memcmp`). - The client never references `boost::asio::ssl::rfc2818_verification`; it uses the modern `host_name_verification` (`hazelcast/src/hazelcast/util/util.cpp`). The broken `.ipp` was being dragged in transitively through the umbrella `` include in three internal socket headers. - Replaced the umbrella include in `BaseSocket.h`, `SSLSocket.h`, and `SocketFactory.h` with the specific subheaders the codebase actually uses (`context.hpp`, `error.hpp`, `host_name_verification.hpp`, `stream.hpp`) — matching the include pattern already used in `util.cpp`. This avoids `rfc2818_verification.hpp` entirely. ## Root cause In Boost 1.83.0, `` lists `` among its includes. In Boost.Asio's default header-only mode that pulls in `rfc2818_verification.ipp`, whose hostname-matching code dereferences `ASN1_STRING` fields directly (`domain->data`, `domain->length`). OpenSSL 3.0 made `ASN1_STRING` an opaque type whose layout is no longer exposed by the public headers, so MSVC refuses to compile the field accesses (hence `C2027`; the `C2660` is a follow-on because `domain->data` collapses to a non-pointer expression). GCC/Clang on Linux/macOS happen to accept the opaque-struct dereference more leniently on this path, which is why only the Windows + MSVC + OpenSSL 3.x combination breaks. Upstream Boost dropped `rfc2818_verification` from `` after 1.83.0; it is no longer present in 1.90.0. That is why only the 1.83.0 matrix entry is affected. Tracking: HZ-5424 --- .../hazelcast/client/internal/socket/BaseSocket.h | 10 +++++++++- .../hazelcast/client/internal/socket/SSLSocket.h | 8 +++++++- .../hazelcast/client/internal/socket/SocketFactory.h | 8 +++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/hazelcast/include/hazelcast/client/internal/socket/BaseSocket.h b/hazelcast/include/hazelcast/client/internal/socket/BaseSocket.h index ecc7184fb6..bd8392358f 100644 --- a/hazelcast/include/hazelcast/client/internal/socket/BaseSocket.h +++ b/hazelcast/include/hazelcast/client/internal/socket/BaseSocket.h @@ -19,7 +19,15 @@ #include #include #ifdef HZ_BUILD_WITH_SSL -#include +// Avoid because in Boost <= 1.83 it transitively +// includes rfc2818_verification, whose .ipp dereferences private +// ASN1_STRING fields that became opaque in OpenSSL 3.x and breaks the +// MSVC build. We use the modern host_name_verification instead, so pull +// in only the specific subheaders we actually need. +#include +#include +#include +#include #endif #include "hazelcast/client/socket.h" diff --git a/hazelcast/include/hazelcast/client/internal/socket/SSLSocket.h b/hazelcast/include/hazelcast/client/internal/socket/SSLSocket.h index 2051b299ea..77079587aa 100644 --- a/hazelcast/include/hazelcast/client/internal/socket/SSLSocket.h +++ b/hazelcast/include/hazelcast/client/internal/socket/SSLSocket.h @@ -18,7 +18,13 @@ #ifdef HZ_BUILD_WITH_SSL #include -#include +// See BaseSocket.h: in Boost <= 1.83 transitively +// pulls in rfc2818_verification, which fails to compile against +// OpenSSL 3.x (opaque ASN1_STRING). Use specific subheaders instead. +#include +#include +#include +#include #include "hazelcast/client/internal/socket/BaseSocket.h" diff --git a/hazelcast/include/hazelcast/client/internal/socket/SocketFactory.h b/hazelcast/include/hazelcast/client/internal/socket/SocketFactory.h index 0e1fc07e9d..5d04b1f7e9 100644 --- a/hazelcast/include/hazelcast/client/internal/socket/SocketFactory.h +++ b/hazelcast/include/hazelcast/client/internal/socket/SocketFactory.h @@ -22,7 +22,13 @@ #ifdef HZ_BUILD_WITH_SSL #include -#include +// See BaseSocket.h: in Boost <= 1.83 transitively +// pulls in rfc2818_verification, which fails to compile against +// OpenSSL 3.x (opaque ASN1_STRING). Use specific subheaders instead. +#include +#include +#include +#include #endif // HZ_BUILD_WITH_SSL From 1732eedf387a34e1f9a4e6a6b66478fcf00be3c9 Mon Sep 17 00:00:00 2001 From: ihsan demir Date: Tue, 2 Jun 2026 09:09:16 +0000 Subject: [PATCH 2/3] Fix SqlTest.select Windows flake: move bounds-check EXPECT_THROWs outside the row loop [HZ-5424] (#1471) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1470 ## Summary - `SqlTest.select` was iterating over 4 096 SQL rows and calling `EXPECT_THROW` three times per row to verify `get_object()` raises `index_out_of_bounds` / `illegal_argument`. That is **~12 000 C++ exception throws** per test run. - On Windows the CI step runs ProcDump as a debugger (`procdump -e -ma -w client_test.exe`). Every C++ `throw` raises SEH code `0xE06D7363`; the OS delivers this as a *first-chance debug event* to ProcDump **before** the C++ `catch` handler can run. ProcDump logs `[HH:MM:SS]Exception: E06D7363.?AVindex_out_of_bounds@...`, then calls `ContinueDebugEvent()` — **two mandatory kernel-mode round-trips per throw**. 12 000 throws × 2 = 24 000 kernel transitions, combined with 4 096 synchronous `map->get()` server calls, made the loop slow enough for the server-side connection health-check to fire and cancel the SQL cursor (`Client cannot be reached`). - Fix: fetch the first page before the main loop, run the three `EXPECT_THROW` checks on its first row only, then iterate all pages in a `for(;;)` loop starting from the already-fetched first page. Exception count drops from **12 000 to 3**. The `bounds_checked` flag is eliminated. ## Why it was Windows-only Linux/macOS CI does not use ProcDump. Without a debugger attached, `throw` → `catch` is entirely in-process with zero kernel transitions, so the loop completes well within any timeout. ## Root cause detail On Windows, C++ exceptions are built on SEH. When a debugger is attached via `DebugActiveProcess()`, the kernel suspends the target process's threads and delivers an `EXCEPTION_DEBUG_EVENT` to the debugger through the kernel debug port. The debugger must call `WaitForDebugEvent()`, process the event, and call `ContinueDebugEvent()` before the target resumes. This happens for *every* throw, even ones that are immediately caught — ProcDump cannot skip them. Under heavy exception load (12 000 throws), the cumulative stall is enough to trigger server-side timeouts. --- hazelcast/test/src/sql_test.cpp | 40 +++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/hazelcast/test/src/sql_test.cpp b/hazelcast/test/src/sql_test.cpp index 36585c616a..ebb14bf0e6 100644 --- a/hazelcast/test/src/sql_test.cpp +++ b/hazelcast/test/src/sql_test.cpp @@ -1870,8 +1870,34 @@ TEST_F(SqlTest, select) std::unordered_set unique_keys; - for (auto itr = res->iterator(); itr.has_next();) { - auto page = itr.next().get(); + // Fetch the first page up front so we can verify the bounds-checking API + // on a single representative row before the main loop. + // Background: on Windows, C++ exceptions are built on SEH. When ProcDump + // is attached as a debugger (CI step: procdump -e -ma -w client_test.exe), + // every throw raises SEH code 0xE06D7363, which the OS delivers as a + // first-chance debug event to ProcDump before the C++ catch handler runs. + // ProcDump logs the line "[HH:MM:SS]Exception: E06D7363.?AV..." and then + // calls ContinueDebugEvent so execution can proceed. This is a mandatory + // two-kernel-mode-transition round-trip for every single throw. + // With the EXPECT_THROW calls inside the 4096-row loop that is ~12 000 + // round-trips, stalling the loop long enough to trigger connection + // timeouts on the server side (Windows-only flaky failure). + auto itr = res->iterator(); + ASSERT_TRUE(itr.has_next()); + auto page = itr.next().get(); + ASSERT_FALSE(page->rows().empty()); + { + const auto& first_row = page->rows().front(); + EXPECT_THROW(first_row.get_object(-1), + exception::index_out_of_bounds); + EXPECT_THROW( + first_row.get_object(first_row.row_metadata().column_count()), + exception::index_out_of_bounds); + EXPECT_THROW(first_row.get_object("unknown_field"), + exception::illegal_argument); + } + + for (;;) { for (const auto& row : page->rows()) { ASSERT_EQ(row_metadata, res->row_metadata()); @@ -1903,14 +1929,10 @@ TEST_F(SqlTest, select) sql_column_type::varchar, val.varchar_val, row, "varcharVal"); unique_keys.emplace(*key0); - - EXPECT_THROW(row.get_object(-1), - exception::index_out_of_bounds); - EXPECT_THROW(row.get_object(row.row_metadata().column_count()), - exception::index_out_of_bounds); - EXPECT_THROW(row.get_object("unknown_field"), - exception::illegal_argument); } + if (!itr.has_next()) + break; + page = itr.next().get(); } EXPECT_THROW(res->iterator(), exception::illegal_state); From 551dd2c6366136cc4ab5cded0e4c9e1409244e9d Mon Sep 17 00:00:00 2001 From: ihsan demir Date: Thu, 4 Jun 2026 13:03:35 +0000 Subject: [PATCH 3/3] Do not warn on benign double completion of backup-aware invocations [HZ-5459] (#1475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1474 ### Problem Since 5.7.0 a healthy C++ client connected to a multi-member cluster continuously emits `WARNING` lines like: ``` Failed to set the response for invocation. Dropping the response. The state of the promise has already been set., ClientInvocation{ ... operation=map.put ... backup_acks_expected_ = 1, backup_acks_received = 1, ... } ``` A 10-client soak run produced 47,664 of these, all for backup-aware operations with `backup_acks_expected == backup_acks_received`, with no actual failures. ### Root cause For a backup-aware operation the invocation completes once the primary response and the backup ack(s) have arrived. Both `notify_response()` and `notify_backup()` can win the final completion, and there is an inherent race in which both reach `complete()` with the **same** pending response: 1. `notify_response` stages `pending_response_` 2. `notify_backup` (other thread) increments the ack count and completes the promise 3. `notify_response`'s recheck now sees `received == expected`, falls through and calls `complete()` again → `boost::promise_already_satisfied` The future is completed exactly once with the correct value; the duplicate is harmlessly dropped. Only the log line is wrong. The race became routine after the multi-threaded pipeline work (#1412), which made the response and the backup ack run on different threads. This mirrors the Java client (`BaseInvocation.notifyResponse` has the same double-check). The difference is reporting: Java's `AbstractInvocationFuture.warnIfSuspiciousDoubleCompletion` only warns when the already-set value differs from the offered one, so the same-value backup-ack race is silent. The C++ `complete()` warned on every `promise_already_satisfied`. ### Fix `complete()` now mirrors Java: a duplicate completion with the same response (the benign backup-ack race) is logged at `finest`, while a genuinely conflicting value still warns. This also aligns the value path with how `set_exception()` already handles the same condition. ### Tests Added `client_invocation_test` with two cases driving the private completion path via friend access: - benign double completion with the same response → no `WARNING`, one `finest` - conflicting double completion with a different response → one `WARNING` --- .../client/spi/impl/ClientInvocation.h | 8 + hazelcast/src/hazelcast/client/spi.cpp | 24 +++ hazelcast/test/src/HazelcastTests5.cpp | 173 ++++++++++++++++++ 3 files changed, 205 insertions(+) diff --git a/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h b/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h index f97ccf002d..56457e61d7 100644 --- a/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h +++ b/hazelcast/include/hazelcast/client/spi/impl/ClientInvocation.h @@ -40,6 +40,12 @@ class logger; namespace client { class address; +namespace test { +// Exposed for testing only; granted access to private completion internals to +// deterministically reproduce the response/backup-ack completion race. +class client_invocation_test; +} // namespace test + namespace connection { class Connection; } @@ -140,6 +146,8 @@ class HAZELCAST_API ClientInvocation friend std::ostream& operator<<(std::ostream& os, const ClientInvocation& invocation); + friend class ::hazelcast::client::test::client_invocation_test; + boost::promise& get_promise(); bool detect_and_handle_backup_timeout( diff --git a/hazelcast/src/hazelcast/client/spi.cpp b/hazelcast/src/hazelcast/client/spi.cpp index cec33dafc7..53819d82a1 100644 --- a/hazelcast/src/hazelcast/client/spi.cpp +++ b/hazelcast/src/hazelcast/client/spi.cpp @@ -2119,6 +2119,30 @@ ClientInvocation::complete(const std::shared_ptr& msg, try { // TODO: move msg content here? this->invocation_promise_.set_value(*msg); + } catch (boost::promise_already_satisfied& e) { + // The promise is already satisfied. This is an expected, benign race + // between notify_response() (primary response) and notify_backup() (the + // last backup ack): for a backup-aware operation both paths can reach + // complete() with the very same pending_response_. The first one wins + // and the duplicate is harmlessly dropped. + // + // This mirrors Java's AbstractInvocationFuture.warnIfSuspiciousDouble- + // Completion, which only warns when the already-set value differs from + // the offered one. Here the canonical value is pending_response_, so a + // completion with the same message is benign (logged at finest) while a + // genuinely different value remains a warning. + auto pending = pending_response_.load(); + auto level = (!pending || *pending != msg) + ? ::hazelcast::logger::level::warning + : ::hazelcast::logger::level::finest; + if (logger_.enabled(level)) { + logger_.log( + level, + boost::str( + boost::format("Failed to set the response for invocation. " + "Dropping the response. %1%, %2% Response: %3%") % + e.what() % *this % *msg)); + } } catch (std::exception& e) { HZ_LOG(logger_, warning, diff --git a/hazelcast/test/src/HazelcastTests5.cpp b/hazelcast/test/src/HazelcastTests5.cpp index 1e4d59c143..cede13a5c2 100644 --- a/hazelcast/test/src/HazelcastTests5.cpp +++ b/hazelcast/test/src/HazelcastTests5.cpp @@ -13,15 +13,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +#include #include #include #include +#include #include #include #include +#include #include #include +#include #include @@ -46,7 +50,11 @@ #include #include #include +#include +#include #include +#include +#include #include #include #include @@ -4174,6 +4182,171 @@ struct hz_serializer } // namespace client } // namespace hazelcast +namespace hazelcast { +namespace client { +namespace test { + +/** + * Regression tests for the completion of a backup-aware invocation. + * + * For a backup-aware operation both notify_response() (primary response) and + * notify_backup() (last backup ack) can race to call ClientInvocation::complete + * with the very same pending response. The promise can only be satisfied once, + * so the loser of the race hits boost::promise_already_satisfied. This is a + * benign duplicate and must not be reported as a WARNING (it used to flood the + * logs of healthy clients). A double completion carrying a *different* value is + * genuinely suspicious and must still be logged at WARNING. + * + * The double completion only arises from a concurrent interleaving that cannot + * be forced through the public notify()/notify_backup() API, so these tests + * drive the private completion path directly via friend access, after staging + * pending_response_ exactly as notify_response() would. + */ +class client_invocation_test : public ClientTest +{ +protected: + using message_ptr = std::shared_ptr; + using invocation_ptr = std::shared_ptr; + using captured_log = std::pair; + + // A single member is enough and is shared by all cases in this suite; the + // completion logic under test does not depend on cluster state. + static void SetUpTestSuite() + { + member_ = new HazelcastServer{ default_server_factory() }; + } + + static void TearDownTestSuite() + { + delete member_; + member_ = nullptr; + } + + // Stages the canonical pending response, mirroring notify_response(). + static void set_pending_response(const invocation_ptr& invocation, + const message_ptr& msg) + { + invocation->pending_response_.store( + boost::make_shared>(msg)); + } + + // Invokes the private complete(); erase=false so we do not touch the + // invocation registry for this manually created invocation. + static void complete(const invocation_ptr& invocation, + const message_ptr& msg) + { + invocation->complete(msg, false); + } + + // Builds a client whose logger forwards every message (finest and above) + // into the supplied sink. + static hazelcast_client make_capturing_client( + std::mutex& mtx, + std::vector& sink) + { + client_config cfg = get_config(); + cfg.get_logger_config() + .level(logger::level::finest) + .handler([&mtx, &sink](const std::string&, + const std::string&, + logger::level lvl, + const std::string& msg) { + std::lock_guard g{ mtx }; + sink.emplace_back(lvl, msg); + }); + return new_client(std::move(cfg)).get(); + } + + static int count_drop_logs(std::mutex& mtx, + const std::vector& sink, + logger::level lvl) + { + std::lock_guard g{ mtx }; + return static_cast( + std::count_if(sink.begin(), sink.end(), [lvl](const captured_log& e) { + return e.first == lvl && + e.second.find("Failed to set the response") != + std::string::npos; + })); + } + + static HazelcastServer* member_; +}; + +HazelcastServer* client_invocation_test::member_ = nullptr; + +TEST_F(client_invocation_test, + benign_double_completion_with_same_response_is_logged_at_finest) +{ + std::mutex mtx; + std::vector logs; + auto client = make_capturing_client(mtx, logs); + + spi::ClientContext context{ client }; + auto request = protocol::codec::client_ping_encode(); + auto invocation = + spi::impl::ClientInvocation::create(context, request, "ping"); + auto future = invocation->get_promise().get_future(); + + auto response = std::make_shared( + protocol::codec::client_ping_encode()); + + // Reproduce the post-race state: pending_response_ holds the response that + // both the primary-response and the last-backup-ack paths would complete + // with, then let both paths complete with that very same message. + set_pending_response(invocation, response); + complete(invocation, response); // first completion wins + complete(invocation, response); // benign duplicate, must not warn + + // The promise was satisfied exactly once with the response. + ASSERT_EQ(boost::future_status::ready, + future.wait_for(boost::chrono::seconds(0))); + ASSERT_NO_THROW(future.get()); + + ASSERT_EQ(0, count_drop_logs(mtx, logs, logger::level::warning)); + ASSERT_EQ(1, count_drop_logs(mtx, logs, logger::level::finest)); + + client.shutdown().get(); +} + +TEST_F( + client_invocation_test, + suspicious_double_completion_with_different_response_is_logged_at_warning) +{ + std::mutex mtx; + std::vector logs; + auto client = make_capturing_client(mtx, logs); + + spi::ClientContext context{ client }; + auto request = protocol::codec::client_ping_encode(); + auto invocation = + spi::impl::ClientInvocation::create(context, request, "ping"); + auto future = invocation->get_promise().get_future(); + + auto first_response = std::make_shared( + protocol::codec::client_ping_encode()); + auto different_response = std::make_shared( + protocol::codec::client_ping_encode()); + + set_pending_response(invocation, first_response); + complete(invocation, first_response); // first completion wins + complete(invocation, + different_response); // genuinely conflicting, must warn + + ASSERT_EQ(boost::future_status::ready, + future.wait_for(boost::chrono::seconds(0))); + ASSERT_NO_THROW(future.get()); + + ASSERT_EQ(1, count_drop_logs(mtx, logs, logger::level::warning)); + ASSERT_EQ(0, count_drop_logs(mtx, logs, logger::level::finest)); + + client.shutdown().get(); +} + +} // namespace test +} // namespace client +} // namespace hazelcast + namespace std { template<> struct hash