search

kazu-yamamoto / http2

HTTP/2.0 library including HPACK
BSD 3-Clause "New" or "Revised" License
86 stars 23 forks source link

Avoid unnecessary empty data frames at end of stream #140

Closed FinleyMcIlwaine closed 3 months ago

FinleyMcIlwaine commented 3 months ago

If a stream terminates with an empty data frame, we will send that empty data frame even if it is succeeded by trailers that are marked end of stream. That empty data frame is unnecessary. We should only send it if the payload length is non-zero and we are not sending trailers.

I discovered this after seeing too many empty data errors from terminating a server handling several concurrent streams with an exception.

FinleyMcIlwaine commented 3 months ago

I cannot reproduce the current test failure locally after running the test on a loop for a while, on my branch or on main. My guess is that it is a non-deterministic failure that is not associated with these changes.

kazu-yamamoto commented 3 months ago

It seems to me that your logic is correct. So, rebased and merged. Thanks.

FinleyMcIlwaine commented 3 months ago

@kazu-yamamoto Thanks for that! Also, we have another PR incoming soon, so don't worry about releasing these changes just yet.

kazu-yamamoto commented 3 months ago

The previous release was for me. I needed to deploy the new ServerIO for my project.