flow_control: reserve window for single stream, cap multi-stream stalls

[Mygod]

This addresses #30 (comment).

Similar issues might have been around for other projects as well, including slipstream (C) and dnstt (Mygod/dnstt#7). CC @EndPositive

[Copilot]

Pull request overview

This PR implements a dual-mode flow control strategy to address issue #30, specifically preventing stalled streams from blocking new connections while preserving TCP-like backpressure for single-stream transfers.

Changes:

• Introduced single-stream and multi-stream flow control modes with automatic transition when a second stream is detected
• Added per-stream queue caps with STOP_SENDING and data discarding to prevent connection-level stalls in multi-stream mode
• Added configurable environment variables (SLIPSTREAM_STREAM_QUEUE_MAX_BYTES, SLIPSTREAM_CONN_RESERVE_BYTES) to tune flow control behavior

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/protocol.md	Updated backpressure documentation to describe the new dual-mode flow control strategy
docs/design.md	Added comprehensive explanation of flow control goals and implementation approach
docs/config.md	Documented two new environment variables for configuring stream queue limits and connection reserve
crates/slipstream-server/tests/flow_control_e2e.rs	Added E2E tests verifying blocked streams don't stall other streams and single-stream slow transfers work correctly
crates/slipstream-server/src/streams.rs	Core server-side flow control implementation with multi-stream tracking, reserve window calculation, and per-stream caps
crates/slipstream-client/src/streams.rs	Core client-side flow control implementation mirroring server behavior
crates/slipstream-ffi/src/picoquic.rs	Added picoquic_stop_sending FFI binding for stream flow control signaling
crates/slipstream-ffi/src/runtime.rs	Updated comment to reflect new flow control approach

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Mygod]

@codex review

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

crates/slipstream-server/src/streams.rs:192

• When handling stream_reset or stop_sending callbacks, the code calls picoquic_reset_stream again at line 192. However, these callbacks are typically triggered by receiving a RESET_STREAM or STOP_SENDING from the peer, or by our own call to picoquic_reset_stream. Calling picoquic_reset_stream again in response to the callback may be redundant or could potentially cause issues. Consider whether this second reset call is necessary, or if it should only be called in specific circumstances (e.g., only when handling peer-initiated resets, not our own).

        picoquic_call_back_event_t::picoquic_callback_stream_reset
        | picoquic_call_back_event_t::picoquic_callback_stop_sending => {
            let reason = match fin_or_event {
                picoquic_call_back_event_t::picoquic_callback_stream_reset => "stream_reset",
                picoquic_call_back_event_t::picoquic_callback_stop_sending => "stop_sending",
                _ => "unknown",
            };
            let key = StreamKey {
                cnx: cnx as usize,
                stream_id,
            };
            if let Some(stream) = shutdown_stream(state, key) {
                warn!(
                    "stream {:?}: reset event={} tx_bytes={} rx_bytes={} consumed_offset={} queued={} pending_chunks={} pending_fin={} fin_enqueued={} fin_offset={:?} target_fin_pending={} close_after_flush={}",
                    key.stream_id,
                    reason,
                    stream.tx_bytes,
                    stream.flow.rx_bytes,
                    stream.flow.consumed_offset,
                    stream.flow.queued_bytes,
                    stream.pending_data.len(),
                    stream.pending_fin,
                    stream.fin_enqueued,
                    stream.flow.fin_offset,
                    stream.target_fin_pending,
                    stream.close_after_flush
                );
            } else {
                warn!(
                    "stream {:?}: reset event={} (unknown stream)",
                    stream_id, reason
                );
            }
            let _ = picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_FILE_CANCEL_ERROR);

crates/slipstream-client/src/streams.rs:202

• When handling stream_reset or stop_sending callbacks, the code calls picoquic_reset_stream again at line 202. However, these callbacks are typically triggered by receiving a RESET_STREAM or STOP_SENDING from the peer, or by our own call to picoquic_reset_stream. Calling picoquic_reset_stream again in response to the callback may be redundant or could potentially cause issues. Consider whether this second reset call is necessary, or if it should only be called in specific circumstances (e.g., only when handling peer-initiated resets, not our own).

        picoquic_call_back_event_t::picoquic_callback_stream_reset
        | picoquic_call_back_event_t::picoquic_callback_stop_sending => {
            let reason = match fin_or_event {
                picoquic_call_back_event_t::picoquic_callback_stream_reset => "stream_reset",
                picoquic_call_back_event_t::picoquic_callback_stop_sending => "stop_sending",
                _ => "unknown",
            };
            if let Some(stream) = state.streams.remove(&stream_id) {
                warn!(
                    "stream {}: reset event={} rx_bytes={} tx_bytes={} queued={} consumed_offset={} fin_offset={:?} fin_enqueued={}",
                    stream_id,
                    reason,
                    stream.flow.rx_bytes,
                    stream.tx_bytes,
                    stream.flow.queued_bytes,
                    stream.flow.consumed_offset,
                    stream.flow.fin_offset,
                    stream.fin_enqueued
                );
            } else {
                warn!(
                    "stream {}: reset event={} (unknown stream)",
                    stream_id, reason
                );
            }
            let _ = picoquic_reset_stream(cnx, stream_id, SLIPSTREAM_FILE_CANCEL_ERROR);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[chatgpt-codex-connector]

Codex Review: Something went wrong. Try again later by commenting “@codex review”.

Unknown error

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".