Make it possible to guarantee that final DATA frame is marked end-of-stream

[edsko]

Prior to this PR, when the thread constructing the streaming body of a request (that is, the argument to requestStreaming or requestStreamingUnmask) terminated without calling flush, the final DATA frame would usually be marked as end-of-stream, but there was no guarantee that this would happen. Specifically, this depended on whether the StreamingFinished chunk would be enqueued before some other thread interrupted (perhaps because of a window update):

https://github.com/kazu-yamamoto/http2/blob/398a5c5284421399a6b91a12734a2293a78c0a1b/Network/HTTP2/Client/Run.hs#L206-L211

If this chunk happened to be enqueued a bit later, the result would be that the constructed DATA frame would not be marked as end-of-stream, but instead the stream would terminate with a separate empty data frame. Unfortunately, this confuses some servers, so we need a way to have more control over this.

This PR (along with a companion small PR for http2) achieves this goal in a few steps:

• First, we introduce requestStreamingIface. I was worried that we'd end up with a whole family of functions requestStreaming, requestStreamingUnmask, requestStreamingThisThatOrTheOther; so to avoid this, requestStreamingIface instead provides the callback with a record of functions, which we can add new features to at will without breaking backwards compatibility and without having to invent new names.
• Then some refactoring: we document StreamingChunk, split fillBufBuilder (see comments inline for the commits), simplify DynaNext (again, comments in the commit), and refactor fillStreamBodyGetNext (see commit). These changes just pave the way for the "real" change in the next commit.
• We add an argument to StreamingBuilder to indicate that this Builder is the end-of-stream. This is the direct send equivalent of what we did for receive in Mark final chunk as final #1.
• Finally, we add outBodyPushFinal to OutBodyIface, which can then be populated in http2, making use of the new argument to StreamingBuilder.

[kazu-yamamoto]

This PR breaks http3.
I should fix it before releasing a new http-semantics.

[edsko]

Do you mean "break" as in: it needs to be updated because it doesn't compile anymore, or do you mean you updated it and the tests break? The latter would be worrying. If the former, there are three changes that you need to make:

• DynaNext just wants the available room now, rather than the buffer limit and window size separately; in http2 this is a single change:

-            Next datPayloadLen reqflush mnext <- curr datBuf datBufSiz lim
+            Next datPayloadLen reqflush mnext <- curr datBuf (min datBufSiz lim)

• You need to provide the Nothing argument to StreamingBuilder in the server (we could think about introducing the same kind of generalization server-side that we did client-side, but I haven't done that, and so server-side we cannot mark the final data frame as final):

-                atomically $ writeTBQueue tbq (StreamingBuilder b)
+                atomically $ writeTBQueue tbq (StreamingBuilder b Nothing)

• OutBodyStreamingUnmask now wants an OutbodyIface rather than the three separate callbacks:

-        let push b = atomically $ writeTBQueue tbq (StreamingBuilder b)
-            flush = atomically $ writeTBQueue tbq StreamingFlush
-            finished = atomically $ writeTBQueue tbq $ StreamingFinished (decCounter mgr)
+        decrementedCounter <- newIORef False
+        let decCounterOnce = do
+              alreadyDecremented <- atomicModifyIORef decrementedCounter $ \b -> (True, b)
+              unless alreadyDecremented $ decCounter mgr
+        let iface = OutBodyIface {
+                outBodyUnmask = unmask
+              , outBodyPush = \b -> atomically $ writeTBQueue tbq (StreamingBuilder b Nothing)
+              , outBodyPushFinal = \b -> atomically $ writeTBQueue tbq (StreamingBuilder b (Just decCounterOnce))
+              , outBodyFlush = atomically $ writeTBQueue tbq StreamingFlush
+              }
+            finished = atomically $ writeTBQueue tbq $ StreamingFinished decCounterOnce
         incCounter mgr
-        strmbdy unmask push flush `finally` finished
+        strmbdy iface `finally` finished

I think that should be it. Let me know if you'd like me to take a look instead.

[kazu-yamamoto]

The former. Thank you for the diffs. I will try to fix http3 on Monday.

[edsko]

Ok, perfect. Thanks @kazu-yamamoto ! Have a great weekend :)

[kazu-yamamoto]

http3 is also ready.

[kazu-yamamoto]

Merged.
I will release a new version soon.

[edsko]

Thanks @kazu-yamamoto ! Pleasure working with you as always :)