Skip to content

serve quic client data flows on bidirectional streams - #234

Closed
fabracht wants to merge 1 commit into
mainfrom
quic-bidi-client-flows
Closed

fabracht wants to merge 1 commit into
mainfrom
quic-bidi-client-flows

Conversation

@fabracht

Copy link
Copy Markdown
Contributor

Part of #207. #229 refused client data flows on bidirectional streams with ERROR_FLOW_REFUSED. MQoQ §9.20 allows them, and §9.1.2 keeps a flow's acks, PINGRESP and subscription deliveries on that flow. This serves them.

What changes:

  • The broker answers each packet from a bidirectional client data flow on the stream it came in on: PUBACK, PUBREC, PUBREL, PUBCOMP, SUBACK, UNSUBACK and PINGRESP. DISCONNECT and AUTH still go on the control stream.
  • Messages for the flow's subscriptions, retained ones included, are delivered on the flow's stream instead of on a stream the broker opens.
  • A newer stream for the flow takes it over and the old one is reset (§9.14). A stream older than the newest one the flow has had is reset when it shows up, even if the newer one has already ended.
  • A FIN from the client ends the flow. The broker answers with FIN and removes the flow's subscriptions (§9.13).
  • If the client resets the stream instead, the flow keeps its subscriptions. Its QoS 1/2 messages wait for the next stream, up to the client's Receive Maximum, and QoS 0 messages are dropped meanwhile (§9.21.2). Past that limit the broker discards the flow.
  • A flow that ends inside a packet is reset with ERROR_INCOMPLETE_PACKET (§9.13.1).
  • A header with the discard flags followed by data is a normal clean flow. Only a header followed directly by FIN is a discard (§9.16).
  • with_flow_discard_rx is now behind transport-quic. Without it, a build of mqtt5 with broker but no transport-quic fails clippy with dead code.

The design was checked first with a TLA+ model in specs/tla/bidi-client-flow. The README there has the results. The proposed rules hold over the full state space (3,478,866 states), and each alternative fails the invariant meant to catch it.

Tests are in quic_flow_streams.rs. Against main, the 8 new tests fail and the other 8 pass. I also broke the takeover check, the waiting of messages, the reply routing and the retained routing one at a time, and each was caught by its test. One rule has no failing test when broken: a late close from a superseded stream must not detach the flow. The broker stops that stream's reader when it supersedes it, so the late close only happens in a race a test can't set up.

Not covered here, for the next parts of #207:

  • Flow expiry. An aborted flow keeps its state until it is discarded, the connection ends or the flow is reopened.
  • QoS 1/2 messages already written to a stream that was then aborted stay unacknowledged until the client reconnects. MQTT 4.4 only allows resending on reconnect, so resending them on the new stream needs a decision first.

@fabracht fabracht closed this Oct 13, 2026
@fabracht

Copy link
Copy Markdown
Contributor Author

Closing this. Our own client never opens client data flows on bidirectional streams. Every strategy uses unidirectional streams, and the only bidirectional stream it opens after the control stream is the flow discard. #229 already stops the broker from closing the connection on these streams; it refuses them with ERROR_FLOW_REFUSED, which is valid under MQoQ §9.20. Serving them would only matter for some other MQoQ client that uses them, and we don't have one. The branch and the TLA model in specs/tla/bidi-client-flow stay around if that changes. The review also raised a few things this branch would need before it could be merged: retained messages losing their flow target when queued, failed writes holding window slots until reconnect, and a stalled flow stream closing the whole connection.

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.

1 participant