fix(http1): fix rare missed write wakeup on connections v2 #3988
+544
−6
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow up on #3952 which was reverted by #3977 after reports in #3976.
The conceptual problem this is trying to fix is that the dispatch loop may poll the Write side and determine readiness (either
rt::Write::poll_writeorrt::Write::poll_flushare ready), but will returnPoll::Pendingto the executor and stall; if neither write nor flush are pending, then nothing will wake us.The initial fix did not treat the common "unbuffered" implementation where poll_flush always returns
Poll::Ready; I question the correctness of such a response, as my understanding of flush would be that the "the bytes are flushed out and we may write again", but I can't argue with what the current convention is.For testing, in addition to the contrived hyper Stream that triggers the stall from the previous PR (ie:
ReadyOnPollStream), I've also added anUnbufferedStreamto simulate the implementation where flush always returnsPoll::Ready. The test fails as expected with #3952.The additional fix in this PR is to check again if the connection is writable if and only if we are proceeding to loop because we think the connection may be writable (ie: write or flush have returned Ready). If the write is indeed still pending, we can safely return from the loop without scheduling a waker because we know the writer's waker will schedule us to proceed with the write half. This additional check resolves the hot looping issue for unbuffered streams in #3952.