Streaming does not work in the new server arch

[kazu-yamamoto]

@edsko's comment copied from #130:

@kazu-yamamoto I just tried with 80de8dbf9e20c93dc72feab600166c9b2aa7addd , same result as with aa6f979146033629121a54964e258c3a611fed7a . My test setup is that the client does a streaming request to the server; this request arrives, and the serves initiates a streaming response. The client receives the headers, but then nothing. If I insert a call to putStrLn in outBodyPush at https://github.com/kazu-yamamoto/http2/blob/80de8dbf9e20c93dc72feab600166c9b2aa7addd/Network/HTTP2/Server/Worker.hs#L150 then I can see that print statement executed with the data that the client should receive, but the client does not receive anything after the response headers. There is a Wireshark log at https://github.com/well-typed/grapesy/pull/183 where we're tracking updating to the new http2 architecture.

[edsko]

Does the fact that you opened this issue mean that you can reproduce it also?

FWIW, @FinleyMcIlwaine did some digging, and wrote at https://github.com/well-typed/grapesy/pull/183#issuecomment-2221616822

The server is hanging in waitForOutbound called from outboundTrailersMaker, which is called from http2 in fillDataHeaderEnqueueNext. The change in behavior seems to be that http2 is somehow constructing the trailers before grapesy is able to mark the outbound thread as done.

We'll continue to look into this (but I have another problem to sort out first).

[kazu-yamamoto]

No, I cannot reproduce it. Thank you for the clue.

[edsko]

@kazu-yamamoto we figured out what was happening. So http2 defines

type Server = Request -> Aux -> (Response -> [PushPromise] -> IO ()) -> IO ()

Let's call that third argument respond.

• In the "old" architecture, respond returned when the response body was sent, but before the trailers were constructed or sent.
• In the new architecture, this is not the case: respond only returns when the response body is sent and the trailers are constructed (and sent, but this is less relevant for us).
• The TrailersMaker constructed by grapesy was waiting for respond to return (because it wanted to be sure the response body was complete), thus resulting in deadlock in the new architecture.

We changed grapesy in https://github.com/well-typed/grapesy/pull/187 so that the TrailersMaker does wait for the last message to be sent but does not wait for the thread to finish, thus fixing the deadlock.

We have run the full grapesy test suite against the new http2 architecture and all our tests pass :partying_face:

[edsko]

I think we can close this issue! :)

[kazu-yamamoto]

Thank you and sorry for your inconvenience.