Skip to content

fix: recover the collision-stashed publish in MqttState::clean() - #1058

Open
holovskyi wants to merge 1 commit into
bytebeamio:mainfrom
holovskyi:fix-clean-collision-livelock
Open

holovskyi wants to merge 1 commit into
bytebeamio:mainfrom
holovskyi:fix-clean-collision-livelock

Conversation

@holovskyi

Copy link
Copy Markdown

Fixes #1056.

Problem

MqttState::clean() rebuilds pending from outgoing_pub/outgoing_rel and resets collision_ping_count, but never takes self.collision. When the session is lost across a reconnect (clean_session = true, or session_present = false dropping pending), the colliding pkid is never re-acked, so:

  • the stashed QoS ≥ 1 publish is silently lost, and
  • the request branch stays disabled (!collision gate in eventloop.rs), so the client loops CollisionTimeout → reconnect → clean() forever, sending nothing but pings and CONNECTs.

Full analysis in #1056.

Fix

Push the stashed publish into pending during clean() — its outgoing_pub slot is free after the sweep and it already carries a pkid, so it rides the normal retransmission path. Applied to both v4 and v5 state, with a regression test each (clean_recovers_collision_stashed_publish).

On a resumed session the old self-heal path (check_collision on the ack of the colliding pkid) still works; this change only matters when that ack can never arrive.

Tests: cargo test -p rumqttc --lib --no-default-features — 75 passed.

🤖 Generated with Claude Code

clean() rebuilt pending from outgoing_pub/outgoing_rel and reset
collision_ping_count, but never took self.collision. When the session is
lost across a reconnect (clean_session = true, or session_present = false
dropping pending), the colliding pkid is never re-acked, so the stashed
publish is silently lost and the request branch stays disabled: the client
loops CollisionTimeout -> reconnect -> clean() forever, sending nothing but
pings and CONNECTs.

Push the stashed publish into pending during clean() (v4 and v5); its
outgoing_pub slot is free after the sweep and it already carries a pkid.
Adds regression tests for both protocol versions.

Fixes bytebeamio#1056

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

MqttState::clean() does not clear collision — permanent reconnect livelock when the session is lost while a publish is stashed

2 participants