Lingering close in ServerHttpProtocol (was #927)

[asvetlov]

Continuation of #927

What do these changes do?

Hello,

Quoting RFC 7230 :

If a server performs an immediate close of a TCP connection, there is
a significant risk that the client will not be able to read the last
HTTP response. If the server receives additional data from the
client on a fully closed connection, such as another request that was
sent by the client before receiving the server's response, the
server's TCP stack will send a reset packet to the client;
unfortunately, the reset packet might erase the client's
unacknowledged input buffers before they can be read and interpreted
by the client's HTTP parser.

To avoid the TCP reset problem, servers typically close a connection
in stages. First, the server performs a half-close by closing only
the write side of the read/write connection. The server then
continues to read from the connection until it receives a
corresponding close by the client, or until the server is reasonably
certain that its own TCP stack has received the client's
acknowledgement of the packet(s) containing the server's last
response. Finally, the server fully closes the connection.

This is a tentative implementation of a lingering close inspired by those nginx settings

Are there changes in behavior for the user?

The typical use case where the absence of lingering close is worrisome in our application is :

1. The client starts PUTting a sizeable amount of data ;
2. Server replies with a HTTP 401 Unauthorized after parsing the headers ;
3. The connection is closed too early for the client to receive / react to the reply.

It is our experience that some HTTP clients behave badly in front of "early" responses. We believe that having lingerings closes is correct and we are currently investigating how much it will help in practice.

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes

[coveralls]

Coverage decreased (-0.1%) to 98.622% when pulling 2b6389b on msornay-lingering-close into b2138da on master.

[codecov-io]

Current coverage is 97.24% (diff: 85.71%)

Merging #1050 into master will decrease coverage by 0.36%

@@             master      #1050   diff @@
==========================================
  Files            28         28          
  Lines          6518       6530    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       1099       1102     +3   
==========================================
- Hits           6362       6350    -12   
- Misses           83         90     +7   
- Partials         73         90    +17   

Powered by Codecov. Last update b2138da...49475a5

[coveralls]

Coverage decreased (-0.1%) to 98.622% when pulling 2b6389b on msornay-lingering-close into b2138da on master.

[coveralls]

Coverage decreased (-0.1%) to 98.622% when pulling 0f541f0 on msornay-lingering-close into b2138da on master.

[msornay]

Hello, thanks for the rewrite.
Are you considering merging this ? Is there any more work required ?

[asvetlov]

Now the patch is out of sync with master.
Don't know when I'll find a time for returning to the issue.

[asvetlov]

Superseeded by #1478

[lock]

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.