Use condition_variable for interruptible thread sleep in updateLoop#147
Use condition_variable for interruptible thread sleep in updateLoop#147jslauthor wants to merge 1 commit intoCountly:masterfrom
Conversation
Replace std::this_thread::sleep_for() in updateLoop() with std::condition_variable::wait_for() so that _deleteThread() can wake the background thread immediately via notify_one(). This makes stop() return instantly instead of blocking for up to the full update interval (e.g. 30 seconds in production builds). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Sorry, accidentally created this. Will open again if needed! |
|
Okay, I do need it. 😅 This lets us cancel any pending requests so the app can close cleanly. Let me know if you'd like any changes! |
|
Hello @jslauthor 🙌 Thanks a lot for your contribution! We’ll review it carefully and get back to you soon. 🙇 |
There was a problem hiding this comment.
Pull request overview
This PR improves shutdown responsiveness of the SDK’s background updateLoop() by making its sleep interruptible, so stop() doesn’t block for up to the full update interval.
Changes:
- Replace
std::this_thread::sleep_for()withstd::condition_variable::wait_for()inCountly::updateLoop()to allow early wakeups on stop. - Add
stop_cv.notify_one()inCountly::_deleteThread()after setting the stop flag. - Add a
std::condition_variable stop_cvmember and required header include.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/countly.cpp |
Uses stop_cv.wait_for() in updateLoop() and notifies the condition variable during _deleteThread() to wake the worker thread promptly. |
include/countly.hpp |
Adds <condition_variable> include and a stop_cv member to support interruptible waits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| std::unique_lock<std::mutex> 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; |
Summary
std::this_thread::sleep_for()inupdateLoop()withstd::condition_variable::wait_for()stop_cv.notify_one()in_deleteThread()to wake the sleeping thread immediatelyThis makes
stop()return instantly instead of blocking for up to the full update interval (e.g. 30 seconds). Currently,_deleteThread()setsstop_thread = trueand callsthread->join(), but the thread is stuck inside an uninterruptiblesleep_for()and won't check the flag until the sleep finishes.This causes GUI applications to hang for up to 30 seconds on quit when the SDK's static singleton destructor calls
stop().Changes
include/countly.hpp#include <condition_variable>std::condition_variable stop_cvmembersrc/countly.cppupdateLoop(): Replacedsleep_forwithstop_cv.wait_for()usingstop_threadas predicate_deleteThread(): Addedstop_cv.notify_one()after settingstop_thread = trueTest plan
stop()returns instantly instead of blocking