From 057b3dd7532549e16307b5251a79ee6151595891 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Tue, 21 Jul 2026 11:32:22 -0400 Subject: [PATCH 1/9] RDKEMW-22043: Upgrade the watchdog logic to use wait_for --- src/gateway.cpp | 12 +++++++++++- test/unit/gatewayTest.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index 9a1e028..23de2d4 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -492,6 +492,8 @@ class GatewayImpl : public IGateway, private IClientTransport Server server; std::thread watchdogThread; std::atomic watchdogRunning; + std::condition_variable watchdogCv; + std::mutex watchdogMtx; bool legacyRPCv1; std::map rpcv1_eventMap; @@ -604,12 +606,19 @@ class GatewayImpl : public IGateway, private IClientTransport watchdogThread = std::thread( [this]() { + std::unique_lock lock(watchdogMtx); while (watchdogRunning) { - std::this_thread::sleep_for(std::chrono::milliseconds(watchdog_interval_ms)); + if (watchdogCv.wait_for(lock, std::chrono::milliseconds(watchdog_interval_ms), + [this] { return !watchdogRunning; })) + { + break; + } try { + lock.unlock(); client.checkPromises(); + lock.lock(); } catch (const std::exception& e) { @@ -643,6 +652,7 @@ class GatewayImpl : public IGateway, private IClientTransport } if (watchdogRunning.exchange(false)) { + watchdogCv.notify_all(); FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] waiting for watchdog thread join..."); auto t0_wdog = std::chrono::steady_clock::now(); if (watchdogThread.joinable()) diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 693117c..49e8cad 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -1505,3 +1505,41 @@ TEST_F(GatewayUTest, DisconnectCancelsPendingRequests) EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error(), Firebolt::Error::NotConnected); } + +// --------------------------------------------------------------------------- +// Test name: GatewayUTest.DisconnectIsNotTimebound +// Covers: disconnect() completes immediately without waiting for watchdog interval +// Scenario type: performance +// --------------------------------------------------------------------------- +TEST_F(GatewayUTest, DisconnectIsNotTimebound) +{ + m_messageHandler = [](connection_hdl, server::message_ptr) {}; + + IGateway& gateway = connectAndWait(); + + // Fire a request that the server will never answer. + auto responseFuture = gateway.request("test.neverResponds", nlohmann::json{}); + + // The request is now in-flight and the future is pending. Disconnect and + // measure the time it takes. With the condition_variable::wait_for refactoring, + // disconnect() should complete immediately (< 100ms) rather than waiting + // for the full watchdog interval (500ms). + auto t0 = std::chrono::steady_clock::now(); + Firebolt::Error disconnectErr = gateway.disconnect(); + auto t1 = std::chrono::steady_clock::now(); + auto disconnectDuration = std::chrono::duration_cast(t1 - t0).count(); + + EXPECT_EQ(disconnectErr, Firebolt::Error::None); + + // Disconnect should complete well within the watchdog interval (500ms). + // Allow some overhead but it should be significantly faster than 500ms. + EXPECT_LT(disconnectDuration, 200) << "disconnect() took " << disconnectDuration + << "ms, expected < 200ms (watchdog interval is 500ms)"; + + // After disconnect() returns, cancelAll() must have resolved the promise. + ASSERT_EQ(responseFuture.wait_for(std::chrono::milliseconds(0)), std::future_status::ready); + + auto result = responseFuture.get(); + EXPECT_FALSE(result.has_value()); + EXPECT_EQ(result.error(), Firebolt::Error::NotConnected); +} From b2d9dd4ce39e1009f1e7e83a963c8ad0a3395b36 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Tue, 21 Jul 2026 12:09:02 -0400 Subject: [PATCH 2/9] fix: PR Comments --- src/gateway.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index 23de2d4..1f9eab9 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -614,11 +614,10 @@ class GatewayImpl : public IGateway, private IClientTransport { break; } + lock.unlock(); try { - lock.unlock(); - client.checkPromises(); - lock.lock(); + client.checkPromises(); } catch (const std::exception& e) { @@ -628,6 +627,7 @@ class GatewayImpl : public IGateway, private IClientTransport { FIREBOLT_LOG_ERROR("Gateway", "[watchdog] checkPromises() threw unknown exception"); } + lock.lock(); } }); FIREBOLT_LOG_DEBUG("Gateway", "[watchdog] thread started"); From 9a442901c63acd86bead17b2c6c6296b0d7bb6d9 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 10:43:22 -0400 Subject: [PATCH 3/9] Fix: Disconnect on a destructor --- src/gateway.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index 1f9eab9..2db712b 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -517,13 +517,10 @@ class GatewayImpl : public IGateway, private IClientTransport ~GatewayImpl() { - if (watchdogRunning) + std::lock_guard lock(connectionLog_mtx); + if (lastConnectionState) { - watchdogRunning = false; - if (watchdogThread.joinable()) - { - watchdogThread.join(); - } + disconnect(); } } From 39ea4cbcedba012bfb02a3ddafdf211e0136e541 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 10:50:46 -0400 Subject: [PATCH 4/9] fix: whitespaces --- src/gateway.cpp | 4 ++-- test/unit/gatewayTest.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index 2db712b..530ef4e 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -607,14 +607,14 @@ class GatewayImpl : public IGateway, private IClientTransport while (watchdogRunning) { if (watchdogCv.wait_for(lock, std::chrono::milliseconds(watchdog_interval_ms), - [this] { return !watchdogRunning; })) + [this] { return !watchdogRunning; })) { break; } lock.unlock(); try { - client.checkPromises(); + client.checkPromises(); } catch (const std::exception& e) { diff --git a/test/unit/gatewayTest.cpp b/test/unit/gatewayTest.cpp index 49e8cad..3793aca 100644 --- a/test/unit/gatewayTest.cpp +++ b/test/unit/gatewayTest.cpp @@ -1534,7 +1534,7 @@ TEST_F(GatewayUTest, DisconnectIsNotTimebound) // Disconnect should complete well within the watchdog interval (500ms). // Allow some overhead but it should be significantly faster than 500ms. EXPECT_LT(disconnectDuration, 200) << "disconnect() took " << disconnectDuration - << "ms, expected < 200ms (watchdog interval is 500ms)"; + << "ms, expected < 200ms (watchdog interval is 500ms)"; // After disconnect() returns, cancelAll() must have resolved the promise. ASSERT_EQ(responseFuture.wait_for(std::chrono::milliseconds(0)), std::future_status::ready); From 4753882fa82a5fc7266bfd338ea860f0c92ce53b Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 10:55:01 -0400 Subject: [PATCH 5/9] fix: Deadlock conditions for internal disconnect --- src/gateway.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index 530ef4e..d633156 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -517,8 +517,12 @@ class GatewayImpl : public IGateway, private IClientTransport ~GatewayImpl() { - std::lock_guard lock(connectionLog_mtx); - if (lastConnectionState) + bool isConnected = false; + { + std::lock_guard lock(connectionLog_mtx); + isConnected = lastConnectionState; + } + if (isConnected) { disconnect(); } From 6fd89cfb0e439bb0efeb1106d0944721c267358e Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 11:12:32 -0400 Subject: [PATCH 6/9] Fix: connection started --- src/gateway.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index d633156..a44bb7f 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -502,6 +502,7 @@ class GatewayImpl : public IGateway, private IClientTransport std::mutex connectionLog_mtx; bool hasLastConnectionLog{false}; bool lastConnectionState{false}; + bool connectionStarted{false}; Firebolt::Error lastConnectionError{Firebolt::Error::None}; std::chrono::steady_clock::time_point lastConnectionLogTs{}; size_t suppressedConnectionNoticeCount{0}; @@ -517,12 +518,12 @@ class GatewayImpl : public IGateway, private IClientTransport ~GatewayImpl() { - bool isConnected = false; + bool needsDisconnection = false; { std::lock_guard lock(connectionLog_mtx); - isConnected = lastConnectionState; + needsDisconnection = connectionStarted; } - if (isConnected) + if (needsDisconnection) { disconnect(); } @@ -600,6 +601,11 @@ class GatewayImpl : public IGateway, private IClientTransport FIREBOLT_LOG_ERROR("Gateway", "[connect] transport connect failed status=%d", static_cast(status)); return status; } + else + { + std::lock_guard lock(connectionLog_mtx); + connectionStarted = true; + } if (!watchdogRunning.exchange(true)) { From 82bc9564ebbfc455d562ba74dfeac2ce5bd045c3 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 11:30:10 -0400 Subject: [PATCH 7/9] fix: Reset Connection started when disconnect is triggered --- src/gateway.cpp | 59 +++++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index a44bb7f..fc05791 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -657,28 +657,7 @@ class GatewayImpl : public IGateway, private IClientTransport { return status; } - if (watchdogRunning.exchange(false)) - { - watchdogCv.notify_all(); - FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] waiting for watchdog thread join..."); - auto t0_wdog = std::chrono::steady_clock::now(); - if (watchdogThread.joinable()) - { - watchdogThread.join(); - } - FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] watchdog joined in %lld ms", - static_cast(std::chrono::duration_cast( - std::chrono::steady_clock::now() - t0_wdog) - .count())); - } - client.cancelAll(); - FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] stopping notification worker..."); - auto t0_nw = std::chrono::steady_clock::now(); - server.stopNotificationWorker(); - FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] notification worker stopped in %lld ms", - static_cast(std::chrono::duration_cast( - std::chrono::steady_clock::now() - t0_nw) - .count())); + cleanupInternalState(); return Error::None; } @@ -941,6 +920,12 @@ class GatewayImpl : public IGateway, private IClientTransport } } + // Disconnection can also happen from the server, it's necessary to cleanup if this ever happens. + if (!connected) + { + cleanupInternalState(); + } + if (emitNotice) { FIREBOLT_LOG_NOTICE("Gateway", "[connection] state=%s error=%d suppressed_repeats=%zu", @@ -974,6 +959,36 @@ class GatewayImpl : public IGateway, private IClientTransport { return transport.getResponseHeader(headerName); } + + void cleanupInternalState() + { + if (watchdogRunning.exchange(false)) + { + watchdogCv.notify_all(); + FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] waiting for watchdog thread join..."); + auto t0_wdog = std::chrono::steady_clock::now(); + if (watchdogThread.joinable()) + { + watchdogThread.join(); + } + FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] watchdog joined in %lld ms", + static_cast(std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0_wdog) + .count())); + } + client.cancelAll(); + FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] stopping notification worker..."); + auto t0_nw = std::chrono::steady_clock::now(); + server.stopNotificationWorker(); + FIREBOLT_LOG_DEBUG("Gateway", "[disconnect] notification worker stopped in %lld ms", + static_cast(std::chrono::duration_cast( + std::chrono::steady_clock::now() - t0_nw) + .count())); + { + std::lock_guard lock(connectionLog_mtx); + connectionStarted = true; + } + } }; IGateway& GetGatewayInstance() From 50fbcb57bbd9dbcc0acc91865c70c94ea6adef27 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 11:34:19 -0400 Subject: [PATCH 8/9] fix: flag --- src/gateway.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gateway.cpp b/src/gateway.cpp index fc05791..66ef9b4 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -986,7 +986,7 @@ class GatewayImpl : public IGateway, private IClientTransport .count())); { std::lock_guard lock(connectionLog_mtx); - connectionStarted = true; + connectionStarted = false; } } }; From ed399307b4b3e4ec7c6321b77fe848707b96caa6 Mon Sep 17 00:00:00 2001 From: Sathishkumar Deena Kirupakaran Date: Wed, 22 Jul 2026 13:52:49 -0400 Subject: [PATCH 9/9] fix: Cleanup race condition --- src/gateway.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gateway.cpp b/src/gateway.cpp index bc9a1b7..5e985d9 100644 --- a/src/gateway.cpp +++ b/src/gateway.cpp @@ -527,6 +527,7 @@ class GatewayImpl : public IGateway, private IClientTransport Firebolt::Error lastConnectionError{Firebolt::Error::None}; std::chrono::steady_clock::time_point lastConnectionLogTs{}; size_t suppressedConnectionNoticeCount{0}; + std::mutex cleanup_mtx; public: GatewayImpl() @@ -983,6 +984,7 @@ class GatewayImpl : public IGateway, private IClientTransport void cleanupInternalState() { + std::lock_guard lock(cleanup_mtx); if (watchdogRunning.exchange(false)) { watchdogCv.notify_all();