h1 servers can shutdown connections with pending buffered data on filled sockets

[deven96]

Target fix for #4022

The fix I am proposing is to simply propagate pending flushes within the poll loop to give the buffer a chance to get fully flushed and the included test just simply asserts that shutdown was not called with pending flushes

I did notice a previously yanked change on here that modified the poll loop to achieve something similar but this should lead to exiting the poll loop and waiting to get polled again rather than exhausting the CPU

[deven96]

After some discussion, decided to not follow the approach of returning pending from within the poll loop as it could cause us to read less frequently ... given that the bug is explicitly that we shouldn't call shutdown whenever buffered hasn't completed then it probably makes sense to ensure that poll_shutdown for Buffered type completes the flush?

[kornelski]

poll_shutdown in Buffered performing flush seems like the right place to address this.

[seanmonstar]

Sorry for the delay, just took a look.

So, I think ultimately, the poll_loop method is fragile, AND the Write enum should likely get a new Flushing state, so that is_done isn't true until all expected bytes for a message are flushed. Then, poll_shutdown really would be a passdown to the IO transport shutdown. But, because that fragility, that could probably be a much larger and harder change.

In the meantime, this patch does fix it, so I'm inclined to go with it. It looks like CI is angry about an unused method, now.

[deven96]

Sorry for the delay, just took a look.

So, I think ultimately, the poll_loop method is fragile, AND the Write enum should likely get a new Flushing state, so that is_done isn't true until all expected bytes for a message are flushed. Then, poll_shutdown really would be a passdown to the IO transport shutdown. But, because that fragility, that could probably be a much larger and harder change.

In the meantime, this patch does fix it, so I'm inclined to go with it. It looks like CI is angry about an unused method, now.

Thanks for taking a look @seanmonstar . Yes it was the fragility of alternative changes that caused the shutdown instead ... otherwise the best option would be to the poll_loop itself. Willing to look into it in a separate PR if need be

[seanmonstar]

Hm, is the CI failure relevant? Kinda looks that way, but I'm distracted at the moment:

Test output

``` Running tests/ready_on_poll_stream.rs (target/debug/deps/ready_on_poll_stream-d4f86d9d0defa2fd)

running 1 test
2026-06-11T19:58:42.507946Z INFO body_test ThreadId(02) ready_on_poll_stream::h1_server::fixture: Client is reading response...
2026-06-11T19:58:42.508111Z INFO tokio-rt-worker ThreadId(04) ready_on_poll_stream::h1_server::fixture: Creating payload of 16 chunks of 64 KiB each (1 MiB total)...
2026-06-11T19:58:42.508184Z INFO tokio-rt-worker ThreadId(04) ready_on_poll_stream::h1_server::fixture: Server: Sending data response...
test body_test ... FAILED

failures:

---- body_test stdout ----

thread 'body_test' (3627) panicked at tests/h1_server/fixture.rs:105:17:
Chunk timeout: chunk took longer than 200ms
stack backtrace:
0: __rustc::rust_begin_unwind
at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/std/src/panicking.rs:689:5
1: core::panicking::panic_fmt
at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/panicking.rs:80:14
2: ready_on_poll_stream::h1_server::fixture::run::{{closure}}
at ./tests/h1_server/fixture.rs:105:17
3: ready_on_poll_stream::body_test::{{closure}}
at ./tests/ready_on_poll_stream.rs:138:42
4: <core::pin::Pin

as core::future::future::Future>::poll
at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/future/future.rs:133:9
5: tokio::runtime::park::CachedParkThread::block_on::{{closure}}
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/park.rs:284:71
6: tokio::task::coop::with_budget
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/task/coop/mod.rs:167:5
7: tokio::task::coop::budget
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/task/coop/mod.rs:133:5
8: tokio::runtime::park::CachedParkThread::block_on
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/park.rs:284:31
9: tokio::runtime::context::blocking::BlockingRegionGuard::block_on
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/context/blocking.rs:66:14
10: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/scheduler/multi_thread/mod.rs:92:22
11: tokio::runtime::context::runtime::enter_runtime
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/context/runtime.rs:65:16
12: tokio::runtime::scheduler::multi_thread::MultiThread::block_on
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/scheduler/multi_thread/mod.rs:91:9
13: tokio::runtime::runtime::Runtime::block_on_inner
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/runtime.rs:373:50
14: tokio::runtime::runtime::Runtime::block_on
at /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.52.3/src/runtime/runtime.rs:345:18
15: ready_on_poll_stream::body_test
at ./tests/ready_on_poll_stream.rs:138:47
16: ready_on_poll_stream::body_test::{{closure}}
at ./tests/ready_on_poll_stream.rs:134:21
17: core::ops::function::FnOnce::call_once
at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/ops/function.rs:250:5
18: <fn() -> core::result::Result<(), alloc::string::String> as core::ops::function::FnOnce<()>>::call_once
at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.

failures:
body_test

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.73s

</details>

[deven96]

Hm, is the CI failure relevant? Kinda looks that way, but I'm distracted at the moment:

Test output

Yes poll_shutdown previously never called poll_flush but now that it does for the fixture, we just need to early return if there's still pending writes

[seanmonstar]

Right on, thanks for the fix!