Delete HalfClosedLocal

[edsko]

This is the third patch in a patch set of three patches; the first two are #82 and #83, although this PR does not strictly speaking depend on those two PRs (at least in the sense that it can build without).

@kazu-yamamoto , before I explain this PR , a side note: this is the PR I am least sure about, because it's a bit "aggressive": it makes a relatively big change to the library, and it's possible that I've not taken something into account and we need to find a different solution. If that is the case, please let me know.

The motivation for this PR has the same general theme as the motivation for #83. Prior to this PR, the http2 library assumes that once the server closes its write end of a stream, any data frames sent on that stream by the client to the server can silently be ignored. This is perhaps also justified in the web server case: if the only purpose of the input from the client is to tell the server what information to send, then if the server is done sending information, no further input from the client is required.

However, in the general case again this is not ok. My use case is remote procedure calls (gRPC); it's entirely possible that after the server has finished sending information to the client (including HTTP2 trailers), the client might still need to send a lot of information to the server. Indeed, other than "the client is the node that initiates the request", there isn't really a major difference between client and server in gRPC. For example, the client might contact the server, say "I'm going to stream some information to you, after you give me some parameters"; the server might respond with those parameters, and then the client might start streaming that information to the server.

So this PR makes perhaps a bit of an extreme change: it removes the HalfClosedLocal state from http2 entirely. It was already not used on the client side; now it's also not used on the server side (it no longer exists). This means that even when the server closes their end of a stream, any data frames coming from the client are still processed as normal.

I don't think this will cause issues for other use cases, however, I cannot be completely sure; this is where I am a little uncertain about this PR. After all, there must be a reason why the HalfClosedLocal state was introduced in the first place? So, if we need a different solution here, please do let me know.

[kazu-yamamoto]

rior to this PR, the http2 library assumes that once the server closes its write end of a stream

the server closes its read end of a stream?

Also, are you talking about END_STREAM , not about shotdown(2)?

[kazu-yamamoto]

Isn't it good enough to remove this line only?

[kazu-yamamoto]

fce595b added isServer.
I cannot remember why.

[kazu-yamamoto]

We should call halfClosedLocal only when END_STREAM is sent.

[edsko]

the server closes its read end of a stream?

No, that's the thing. It's calling halfClosedLocal when its write end is closed, either when (1) the last output is sent to the client (for streaming), in

http2/Network/HTTP2/Arch/Sender.hs

Line 291 in e41e3f6

when (isServer ctx) $ halfClosedLocal ctx strm Finished

or (2) immediately (for OutBodyNone) in

http2/Network/HTTP2/Arch/Sender.hs

Line 156 in e41e3f6

when (isServer ctx) $ halfClosedLocal ctx strm Finished

The consequence of halfClosedLocal however is that it then ignores input from the client:

http2/Network/HTTP2/Arch/Receiver.hs

Line 389 in e41e3f6

_ctx s@(HalfClosedLocal _)

I think both of these calls to halfClosedLocal are wrong:

• Call (1) to halfClosedLocal means that if I have a server that is streaming data to the client, a client that waits for the server to finish (has sent its trailers), is subsequently unable to communicate with the server
• Call (2) to halfClosedLocal means that if I have a server that uses OutBodyNone to indicate that it has no output at all, and a client that wants to communicate with the server (after the server sent the trailers) is unable to do so.

Since these are the only two calls to halfClosedLocal, removing both of them makes the entire HalfClosedLocal state becomes redundant, which is why I removed it altogether in this PR.

Note that if the client closes their write end of the stream (which is when we're not expecting any output anymore), we will already call halfClosedRemote to record this fact.

[edsko]

Rebased on latest master.

[edsko]

Obsoleted by #90.