Skip to content

fix: bound MQTT shutdown publish waits - #154

Draft
robjarawan wants to merge 2 commits into
developmentfrom
fix/mqtt-close-publish-deadline-refresh
Draft

fix: bound MQTT shutdown publish waits#154
robjarawan wants to merge 2 commits into
developmentfrom
fix/mqtt-close-publish-deadline-refresh

Conversation

@robjarawan

Copy link
Copy Markdown
Owner

Fixes #48.

MQTT.close() waits while pending_publishes is non-empty, but the current loop has no deadline. I reproduced it with the real MQTT v5/QoS 1 publisher and a local Mosquitto broker. The broker completion reached Paho's callback thread, one MID remained in Sarracenia's pending state, and shutdown was still blocked after the bounded 1.5 second probe even with timeout=0.25. The normal completion control closed immediately.

So what I did was bound the drain with a time.monotonic() deadline. Normal completions can still drain during the wait. If the deadline expires, the code logs the unresolved MIDs at error level, increments txBadCount once per unresolved MID, then disconnects and stops the Paho loop. An explicit timeout=0 is honored without sleeping; only a missing or None timeout uses the 300 second default.

The real failure case now returns after 0.252 seconds with MID 1 still visible for diagnosis. The normal broker control still returns immediately.

Check Result
Regression test on current development 2 failed, 1 passed
Regression test after the fix 3 passed
Real local Mosquitto before the fix unresolved MID hung; bounded child terminated after 1.5 seconds
Real local Mosquitto after the fix returned in 0.252 seconds; unresolved MID logged and retained
All tests/sarracenia/moth tests 35 passed
Broader tests without the Azure emulator 357 passed, 1 skipped
Local maintenance fixture from #140 Passed: five messages/files, two queues and three configs removed
Combined locally with #153 38 MQTT tests passed; both real-broker proofs passed
Python compilation and git diff --check Passed

This refresh supersedes the closed fork PR #124. MetPX#1650 remains open upstream and is not part of this fork PR. I kept the disconnected-client path and a dedicated close-timeout option outside this fix because they have separate behavior and test boundaries.

This branch also carries the separate local-fixture CI repair from #140 so the fork checks do not depend on public feeds.

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Test Results

377 tests   376 ✅  1m 44s ⏱️
  1 suites    1 💤
  1 files      0 ❌

Results for commit 30c545d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MQTT close can wait forever for pending publishes

1 participant