Skip to content

Commit a416aa8

Browse files
committed
fix(http1): fix rare missed write wakeup on connections v2
1 parent 2c0c63a commit a416aa8

File tree

1 file changed

+34
-6
lines changed

1 file changed

+34
-6
lines changed

src/proto/h1/dispatch.rs

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,14 @@ where
170170
// benchmarks often use. Perhaps it should be a config option instead.
171171
for _ in 0..16 {
172172
let _ = self.poll_read(cx)?;
173-
let _ = self.poll_write(cx)?;
174-
let _ = self.poll_flush(cx)?;
173+
let write_ready = self.poll_write(cx)?.is_ready();
174+
let flush_ready = self.poll_flush(cx)?.is_ready();
175+
176+
// If we can write more body and the connection is ready, we should
177+
// write again. If we return `Ready(Ok(())` here, we will yield
178+
// without a guaranteed wake-up from the write side of the connection.
179+
// This would lead to a deadlock if we also don't expect reads.
180+
let wants_write_again = self.can_write_again() && (write_ready || flush_ready);
175181

176182
// This could happen if reading paused before blocking on IO,
177183
// such as getting to the end of a framed message, but then
@@ -181,14 +187,31 @@ where
181187
//
182188
// Using this instead of task::current() and notify() inside
183189
// the Conn is noticeably faster in pipelined benchmarks.
184-
if !self.conn.wants_read_again() {
185-
//break;
190+
let wants_read_again = self.conn.wants_read_again();
191+
192+
// If we cannot write or read again, we yield and rely on the
193+
// wake-up from the connection futures.
194+
if !(wants_write_again || wants_read_again) {
186195
return Poll::Ready(Ok(()));
187196
}
188-
}
189197

198+
// If we are continuing only because "wants_write_again", check if write is ready.
199+
if !wants_read_again && wants_write_again {
200+
// If write was ready, just proceed with the loop
201+
if write_ready {
202+
continue;
203+
}
204+
// Write was previously pending, but may have become ready since polling flush, so
205+
// we need to check it again. If we simply proceeded, the case of an unbuffered
206+
// writer where flush is always ready would cause us to hot loop.
207+
if self.poll_write(cx)?.is_pending() {
208+
// write is pending, so it is safe to yield and rely on wake-up from connection
209+
// futures.
210+
return Poll::Ready(Ok(()));
211+
}
212+
}
213+
}
190214
trace!("poll_loop yielding (self = {:p})", self);
191-
192215
task::yield_now(cx).map(|never| match never {})
193216
}
194217

@@ -433,6 +456,11 @@ where
433456
self.conn.close_write();
434457
}
435458

459+
/// If there is pending data in body_rx, we can make progress writing if the connection is ready.
460+
fn can_write_again(&mut self) -> bool {
461+
self.body_rx.is_some()
462+
}
463+
436464
fn is_done(&self) -> bool {
437465
if self.is_closing {
438466
return true;

0 commit comments

Comments
 (0)