diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c1bf66..9c6683b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## XX.XX.XX +- Added `enableImmediateRequestOnStop` configuration option. When enabled, the update loop uses a condition variable instead of polling, allowing `stop()` and `setUpdateInterval()` to take effect immediately rather than waiting for the current sleep interval to expire. + ## 23.2.4 - Mitigated an issue where cached events were not queued when a user property was recorded. diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d72104..5f1a6d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -113,7 +113,8 @@ if(COUNTLY_BUILD_TESTS) ${CMAKE_CURRENT_SOURCE_DIR}/tests/event.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/crash.cpp ${CMAKE_CURRENT_SOURCE_DIR}/tests/request.cpp - ${CMAKE_CURRENT_SOURCE_DIR}/tests/config.cpp) + ${CMAKE_CURRENT_SOURCE_DIR}/tests/config.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/tests/immediate_stop.cpp) target_compile_options(countly-tests PRIVATE -g) target_compile_definitions(countly-tests PRIVATE COUNTLY_BUILD_TESTS) diff --git a/include/countly.hpp b/include/countly.hpp index 6065b9d..2bd4fb8 100644 --- a/include/countly.hpp +++ b/include/countly.hpp @@ -5,6 +5,7 @@ #include "countly/countly_configuration.hpp" #include +#include #include #include #include @@ -60,6 +61,8 @@ class Countly : public cly::CountlyDelegates { void disableAutoEventsOnUserProperties(); + void enableImmediateRequestOnStop(); + void setHTTPClient(HTTPClientFunction fun); void setMetrics(const std::string &os, const std::string &os_version, const std::string &device, const std::string &resolution, const std::string &carrier, const std::string &app_version); @@ -351,6 +354,7 @@ class Countly : public cly::CountlyDelegates { bool enable_automatic_session = false; bool stop_thread = false; bool running = false; + std::condition_variable stop_cv; // Wakes updateLoop immediately on stop size_t wait_milliseconds = COUNTLY_KEEPALIVE_INTERVAL; size_t max_events = COUNTLY_MAX_EVENTS_DEFAULT; diff --git a/include/countly/countly_configuration.hpp b/include/countly/countly_configuration.hpp index 6540338..09b7c1e 100644 --- a/include/countly/countly_configuration.hpp +++ b/include/countly/countly_configuration.hpp @@ -72,6 +72,13 @@ struct CountlyConfiguration { bool autoEventsOnUserProperties = true; + /** + * Enable immediate stop notification using a condition variable. + * When enabled, the update loop wakes immediately on stop instead of + * waiting for the current sleep interval to expire. + */ + bool immediateRequestOnStop = false; + HTTPClientFunction http_client_function = nullptr; nlohmann::json metrics; diff --git a/src/countly.cpp b/src/countly.cpp index bf01fef..0fa1772 100644 --- a/src/countly.cpp +++ b/src/countly.cpp @@ -156,6 +156,16 @@ void Countly::disableAutoEventsOnUserProperties() { mutex->unlock(); } +void Countly::enableImmediateRequestOnStop() { + if (is_sdk_initialized) { + log(LogLevel::WARNING, "[Countly][enableImmediateRequestOnStop] You can not enable immediate request on stop after SDK initialization."); + return; + } + + std::lock_guard lk(*mutex); + configuration->immediateRequestOnStop = true; +} + void Countly::setMetrics(const std::string &os, const std::string &os_version, const std::string &device, const std::string &resolution, const std::string &carrier, const std::string &app_version) { if (is_sdk_initialized) { log(LogLevel::WARNING, "[Countly][setMetrics] You can not set metrics after SDK initialization."); @@ -501,9 +511,13 @@ void Countly::stop() { } void Countly::_deleteThread() { - mutex->lock(); - stop_thread = true; - mutex->unlock(); + { + std::lock_guard lk(*mutex); + stop_thread = true; + } + if (configuration->immediateRequestOnStop) { + stop_cv.notify_one(); + } if (thread && thread->joinable()) { try { thread->join(); @@ -515,9 +529,13 @@ void Countly::_deleteThread() { } void Countly::setUpdateInterval(size_t milliseconds) { - mutex->lock(); - wait_milliseconds = milliseconds; - mutex->unlock(); + { + std::lock_guard lk(*mutex); + wait_milliseconds = milliseconds; + } + if (configuration->immediateRequestOnStop) { + stop_cv.notify_one(); + } } void Countly::addEvent(const cly::Event &event) { @@ -1213,29 +1231,58 @@ std::chrono::system_clock::duration Countly::getSessionDuration() { return Count void Countly::updateLoop() { log(LogLevel::DEBUG, "[Countly][updateLoop]"); - mutex->lock(); - running = true; - mutex->unlock(); - while (true) { - mutex->lock(); - if (stop_thread) { - stop_thread = false; + { + std::lock_guard lk(*mutex); + running = true; + } + if (configuration->immediateRequestOnStop) { + try { + while (true) { + { + std::unique_lock lk(*mutex); + stop_cv.wait_for(lk, std::chrono::milliseconds(wait_milliseconds), [this] { + return stop_thread; + }); + if (stop_thread) { + stop_thread = false; + running = false; + return; + } + } + if (enable_automatic_session == true && configuration->manualSessionControl == false) { + updateSession(); + } else if (configuration->manualSessionControl == true) { + packEvents(); + } + requestModule->processQueue(mutex); + } + } catch (...) { + std::lock_guard lk(*mutex); + running = false; + log(LogLevel::ERROR, "[Countly][updateLoop] unexpected exception, stopping update loop"); + } + } else { + while (true) { + mutex->lock(); + if (stop_thread) { + stop_thread = false; + mutex->unlock(); + break; + } + size_t last_wait_milliseconds = wait_milliseconds; mutex->unlock(); - break; + std::this_thread::sleep_for(std::chrono::milliseconds(last_wait_milliseconds)); + if (enable_automatic_session == true && configuration->manualSessionControl == false) { + updateSession(); + } else if (configuration->manualSessionControl == true) { + packEvents(); + } + requestModule->processQueue(mutex); } - size_t last_wait_milliseconds = wait_milliseconds; + mutex->lock(); + running = false; mutex->unlock(); - std::this_thread::sleep_for(std::chrono::milliseconds(last_wait_milliseconds)); - if (enable_automatic_session == true && configuration->manualSessionControl == false) { - updateSession(); - } else if (configuration->manualSessionControl == true) { - packEvents(); - } - requestModule->processQueue(mutex); } - mutex->lock(); - running = false; - mutex->unlock(); } void Countly::enableRemoteConfig() { diff --git a/src/storage_module_db.cpp b/src/storage_module_db.cpp index 60c73e9..5020503 100644 --- a/src/storage_module_db.cpp +++ b/src/storage_module_db.cpp @@ -172,7 +172,7 @@ void StorageModuleDB::RQRemoveFront(std::shared_ptr request) { } // Log the request ID being removed - _logger->log(LogLevel::DEBUG, "[Countly][StorageModuleDB] RQRemoveFront RequestID = " + request->getId()); + _logger->log(LogLevel::DEBUG, "[Countly][StorageModuleDB] RQRemoveFront RequestID = " + std::to_string(request->getId())); #ifdef COUNTLY_USE_SQLITE sqlite3 *database; diff --git a/tests/config.cpp b/tests/config.cpp index 002a590..c17c006 100644 --- a/tests/config.cpp +++ b/tests/config.cpp @@ -52,6 +52,7 @@ TEST_CASE("Validate setting configuration values") { CHECK(config.forcePost == false); CHECK(config.port == 443); CHECK(config.manualSessionControl == false); + CHECK(config.immediateRequestOnStop == false); CHECK(config.sha256_function == nullptr); CHECK(config.http_client_function == nullptr); CHECK(config.metrics.empty()); @@ -78,6 +79,7 @@ TEST_CASE("Validate setting configuration values") { ct.SetPath(TEST_DATABASE_NAME); ct.setMaxRQProcessingBatchSize(10); ct.enableManualSessionControl(); + ct.enableImmediateRequestOnStop(); ct.start("YOUR_APP_KEY", "https://try.count.ly", -1, false); // Get configuration values using Countly getters @@ -97,6 +99,7 @@ TEST_CASE("Validate setting configuration values") { CHECK(config.forcePost == true); CHECK(config.port == 443); CHECK(config.manualSessionControl == true); + CHECK(config.immediateRequestOnStop == true); CHECK(config.sha256_function("custom SHA256") == customSha_1_returnValue); HTTPResponse response = config.http_client_function(true, "", ""); @@ -182,6 +185,7 @@ TEST_CASE("Validate setting configuration values") { ct.setSalt("new-salt"); ct.setMaxRequestQueueSize(100); ct.SetPath("new_database.db"); + ct.enableImmediateRequestOnStop(); // get SDK configuration again and make sure that they haven't changed config = ct.getConfiguration(); @@ -199,6 +203,7 @@ TEST_CASE("Validate setting configuration values") { CHECK(config.breadcrumbsThreshold == 100); CHECK(config.forcePost == true); CHECK(config.port == 443); + CHECK(config.immediateRequestOnStop == false); // was never enabled before init, should stay false CHECK(config.sha256_function("custom SHA256") == customSha_1_returnValue); response = config.http_client_function(true, "", ""); diff --git a/tests/immediate_stop.cpp b/tests/immediate_stop.cpp new file mode 100644 index 0000000..d2ee058 --- /dev/null +++ b/tests/immediate_stop.cpp @@ -0,0 +1,190 @@ +#include "countly.hpp" +#include "doctest.h" +#include "nlohmann/json.hpp" +#include "test_utils.hpp" +#include +#include + +using namespace cly; +using namespace test_utils; + +/** + * Integration tests for the immediateRequestOnStop feature. + * + * These tests verify that the condition-variable-based update loop + * behaves correctly end-to-end: session lifecycle, event delivery, + * manual session control, and immediate shutdown responsiveness. + * A separate test case verifies the fallback (sleep-based) path. + */ + +// Helper: search http_call_queue for a request containing a specific key=value pair +static bool httpQueueContains(const std::string &key, const std::string &value) { + for (const auto &call : http_call_queue) { + auto it = call.data.find(key); + if (it != call.data.end() && it->second == value) { + return true; + } + } + return false; +} + +// Helper: search http_call_queue for a request containing a specific event key +static bool httpQueueContainsEvent(const std::string &event_key) { + for (const auto &call : http_call_queue) { + auto it = call.data.find("events"); + if (it != call.data.end()) { + nlohmann::json events = nlohmann::json::parse(it->second); + for (const auto &e : events) { + if (e["key"].get() == event_key) { + return true; + } + } + } + } + return false; +} + +TEST_CASE("immediateRequestOnStop - session lifecycle through CV loop") { + clearSDK(); + Countly &ct = Countly::getInstance(); + ct.setHTTPClient(fakeSendHTTP); + ct.setDeviceID(COUNTLY_TEST_DEVICE_ID); + ct.SetPath(TEST_DATABASE_NAME); + ct.enableImmediateRequestOnStop(); + ct.setAutomaticSessionUpdateInterval(1); + http_call_queue.clear(); + + ct.start(COUNTLY_TEST_APP_KEY, COUNTLY_TEST_HOST, COUNTLY_TEST_PORT, true); + // Wait for the loop to process the begin request + std::this_thread::sleep_for(std::chrono::seconds(2)); + // Flush any remaining RQ items through our fakeSendHTTP + ct.processRQDebug(); + + // Verify session begin was sent + CHECK(httpQueueContains("begin_session", "1")); + + // Now stop and verify session end + http_call_queue.clear(); + ct.stop(); + ct.processRQDebug(); + + CHECK(httpQueueContains("end_session", "1")); +} + +TEST_CASE("immediateRequestOnStop - event delivery through CV loop") { + clearSDK(); + Countly &ct = Countly::getInstance(); + ct.setHTTPClient(fakeSendHTTP); + ct.setDeviceID(COUNTLY_TEST_DEVICE_ID); + ct.SetPath(TEST_DATABASE_NAME); + ct.enableImmediateRequestOnStop(); + ct.setAutomaticSessionUpdateInterval(1); + http_call_queue.clear(); + + ct.start(COUNTLY_TEST_APP_KEY, COUNTLY_TEST_HOST, COUNTLY_TEST_PORT, true); + + // Add events after the loop is running + cly::Event event1("purchase", 1); + ct.addEvent(event1); + cly::Event event2("login", 1); + ct.addEvent(event2); + + // Wait for the threaded update loop to pick up and process events + // With 1-second interval, 3 seconds gives at least 2 full cycles + std::this_thread::sleep_for(std::chrono::seconds(3)); + ct.stop(); + + CHECK(httpQueueContainsEvent("purchase")); + CHECK(httpQueueContainsEvent("login")); +} + +TEST_CASE("immediateRequestOnStop - stop responsiveness with long interval") { + clearSDK(); + Countly &ct = Countly::getInstance(); + ct.setHTTPClient(fakeSendHTTP); + ct.setDeviceID(COUNTLY_TEST_DEVICE_ID); + ct.SetPath(TEST_DATABASE_NAME); + ct.enableImmediateRequestOnStop(); + // Use a long update interval to prove the CV wakes the thread, not the timeout + ct.setAutomaticSessionUpdateInterval(60); + http_call_queue.clear(); + + ct.start(COUNTLY_TEST_APP_KEY, COUNTLY_TEST_HOST, COUNTLY_TEST_PORT, true); + // Let the thread enter wait_for with the 60-second interval + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + auto before = std::chrono::steady_clock::now(); + ct.stop(); + auto elapsed_ms = std::chrono::duration_cast( + std::chrono::steady_clock::now() - before) + .count(); + + // Must complete well under the 60-second interval. + // A generous 5-second threshold avoids CI flakiness while still + // proving the CV woke the thread (60s vs <5s is unambiguous). + CHECK(elapsed_ms < 5000); + + // Verify the session was still properly ended despite the immediate stop + ct.processRQDebug(); + CHECK(httpQueueContains("end_session", "1")); +} + +TEST_CASE("immediateRequestOnStop - manual session control through CV loop") { + clearSDK(); + Countly &ct = Countly::getInstance(); + ct.setHTTPClient(fakeSendHTTP); + ct.setDeviceID(COUNTLY_TEST_DEVICE_ID); + ct.SetPath(TEST_DATABASE_NAME); + ct.enableImmediateRequestOnStop(); + ct.enableManualSessionControl(); + ct.setAutomaticSessionUpdateInterval(1); + http_call_queue.clear(); + + ct.start(COUNTLY_TEST_APP_KEY, COUNTLY_TEST_HOST, COUNTLY_TEST_PORT, true); + + // In manual session mode, the loop calls packEvents() instead of updateSession() + cly::Event event("manual_event", 5); + ct.addEvent(event); + + // Wait for the thread to pack events (cycle 1) and send them via HTTP (cycle 2). + // With a 1-second interval, 5 seconds gives enough margin. + std::this_thread::sleep_for(std::chrono::seconds(5)); + ct.stop(); + // Flush any remaining items from the RQ + ct.processRQDebug(); + + // Events should be packed and delivered + CHECK(httpQueueContainsEvent("manual_event")); + + // No automatic session begin should have been sent + CHECK_FALSE(httpQueueContains("begin_session", "1")); +} + +TEST_CASE("immediateRequestOnStop - fallback sleep path") { + clearSDK(); + Countly &ct = Countly::getInstance(); + ct.setHTTPClient(fakeSendHTTP); + ct.setDeviceID(COUNTLY_TEST_DEVICE_ID); + ct.SetPath(TEST_DATABASE_NAME); + // Do NOT enable immediateRequestOnStop -- exercises the old sleep_for path + ct.setAutomaticSessionUpdateInterval(1); + http_call_queue.clear(); + + ct.start(COUNTLY_TEST_APP_KEY, COUNTLY_TEST_HOST, COUNTLY_TEST_PORT, true); + + // Add an event while the loop is running + cly::Event event("fallback_event", 3); + ct.addEvent(event); + + // With 1-second interval, 3 seconds gives enough cycles to process + std::this_thread::sleep_for(std::chrono::seconds(3)); + ct.stop(); + ct.processRQDebug(); + + // Verify event delivery works through the fallback path + CHECK(httpQueueContainsEvent("fallback_event")); + + // Verify session lifecycle works through the fallback path + CHECK(httpQueueContains("begin_session", "1")); + CHECK(httpQueueContains("end_session", "1")); +}