Fix MQTT disconnect crash: lifetime & race fixes in MQTT/MoonRaker/HttpServer - #870
Merged
zhangzhend0ng merged 4 commits intoSep 17, 2026
Merged
Conversation
SukiSunYuhang
force-pushed
the
feature_sentry_crash_fix
branch
from
September 17, 2026 03:43
f716d44 to
1d53ceb
Compare
SukiSunYuhang
force-pushed
the
feature_sentry_crash_fix
branch
from
September 17, 2026 04:12
1d53ceb to
a4c08cf
Compare
SukiSunYuhang
added a commit
that referenced
this pull request
Sep 17, 2026
…de m_server_mtx Address review findings on PR #870: - start_http_server is written by the io thread's exception handler without holding m_server_mtx and read unlocked by is_started()/ set_port() and the health-check loop, so it must be atomic. - Split start() into start_locked() plus tail: restart() and start() now call start_health_check() only after m_server_mtx is released. A retired health-check thread can still be blocked in is_healthy() waiting for that lock; joining it via start_health_check() while holding the lock deadlocks (T1 holds m_server_mtx waiting on join, T2 waits on m_server_mtx to finish).
SukiSunYuhang
added a commit
that referenced
this pull request
Sep 17, 2026
…ure in failure callback Address second-round review findings on PR #870: - Moonraker: time_sync_manager_ is reset by disconnect() on the calling thread while Paho receive-thread callbacks and async_* APIs on arbitrary threads read it. All reads now go through a locked time_sync_manager_snapshot(), the disconnect reset happens under the same mutex, and SetMessageCallback(nullptr) is moved BEFORE the reset so no new dispatch can start once the manager is dropped. - SSWCP: the connection failure callback captured the engine shared_ptr while being stored inside the engine itself, keeping its refcount >= 1 forever — ~MqttClient (the only place the callback is cleared) could never run. Capture a weak_ptr instead.
MQTT: - Add MqttClient::create() factory that caches the client's own weak_ptr (self_) at construction; Paho callbacks use it instead of shared_from_this(), which threw bad_weak_ptr for raw new-ed clients and during destruction. A tearing_down_ flag guards late callbacks. - SSWCP sw_get_pin_code: keep the one-shot pin-code client in a static shared_ptr released on the UI thread only (was raw new/delete racing the Paho receive thread); parse the response with allow_exceptions= false so a malformed payload cannot terminate the process inside the C callback. - Moonraker_Mqtt: check callback-map emptiness under m_cbs_mtx in on_status_arrived/on_notification_arrived. HttpServer: - Create the IOServer on the calling thread; the io thread no longer writes server_, which raced the locked readers. - Guard start_health_check against assigning to our own joinable thread handle (std::terminate) when stop() disabled the check while the health thread was inside restart() -> start(). - Hold m_server_mtx across restart() teardown and start() so is_healthy() cannot observe the torn-down state and trigger a second restart.
…de m_server_mtx Address review findings on PR #870: - start_http_server is written by the io thread's exception handler without holding m_server_mtx and read unlocked by is_started()/ set_port() and the health-check loop, so it must be atomic. - Split start() into start_locked() plus tail: restart() and start() now call start_health_check() only after m_server_mtx is released. A retired health-check thread can still be blocked in is_healthy() waiting for that lock; joining it via start_health_check() while holding the lock deadlocks (T1 holds m_server_mtx waiting on join, T2 waits on m_server_mtx to finish).
…ure in failure callback Address second-round review findings on PR #870: - Moonraker: time_sync_manager_ is reset by disconnect() on the calling thread while Paho receive-thread callbacks and async_* APIs on arbitrary threads read it. All reads now go through a locked time_sync_manager_snapshot(), the disconnect reset happens under the same mutex, and SetMessageCallback(nullptr) is moved BEFORE the reset so no new dispatch can start once the manager is dropped. - SSWCP: the connection failure callback captured the engine shared_ptr while being stored inside the engine itself, keeping its refcount >= 1 forever — ~MqttClient (the only place the callback is cleared) could never run. Capture a weak_ptr instead.
SukiSunYuhang
force-pushed
the
feature_sentry_crash_fix
branch
from
September 17, 2026 10:01
89a123b to
6faa813
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the crash (0xC0000409 / std::bad_weak_ptr) triggered when a printer disconnects, by resolving lifetime races across the connection layer:
Utils/MQTT.cpp/.hpp): safe callback dispatch and teardown around disconnectUtils/MoonRaker.cpp/.hpp): eliminate use-after-free on connection dropGUI/HttpServer.cpp/.hpp): harden shutdown against in-flight handlersGUI/SSWCP.cpp/.hpp): related lifetime guardsCommits (rebased linearly on the current
build_platform_engineeringtip):Test plan