Support http connection keep-alive

[barrylb]

Supporting HTTP 1.1 Connection: Keep-Alive is essentially a matter of just not closing the connection after sending a response, and ensuring processing resumes when further data arrives over the connection.

With this modification, a 'keepAlive' parameter is added to the Response end function. The caller can choose to keep the connection open by specifying keepAlive = true. Previous behaviour of closing the connection is retained if the parameter is not specified.

Testing performance with small HTTP POST messages sent using a test program (using php curl) to a test server:

• Closing connection each time: 350 requests per second.
• Persistent connection: 3125 requests per second.

[cboden]

Can you indent with 4 spaces please?

[barrylb]

The latest change addresses the concern about resource cleanup. The RequestHeaderParser needs to exist for the duration of the connection so it can parse multiple HTTP requests for a single persistent connection. The cleanup explicitly removes RequestHeaderParser listeners when the connection ends.

[WyriHaximus]

LGTM 👍

[clue]

Can you give some example code on how you expect this to be used?

It's my understanding that the Response object should behave like a WritableStreamInterface, i.e. end()ing this only terminates its "virtual stream" and should not have any effect on the underlying socket connection. IMO it should be up to the server to determine when the close the underlying connection.

Any info on how other HTTP libs handle this?

[barrylb]

Yeah it could be neater for the http framework to handle closing connections rather than leaving each request handler do it.

The Request object could process the received headers according to http://tools.ietf.org/html/rfc7230#section-6.3 to know when to close a connection. In addition the writeHead function would need to check for presence of a "Connection: Close" header in the response, to close the connection when present.

This would seem to mimic the behaviour of PHP scripts running under Apache.

[HonzaMac]

Hi guys, is there any reason to not merge this PR now?

[WyriHaximus]

Merge conflicts plus I have to finish the work I started on a refactor. After that I'll pull this into a new PR by cherry picking the commits and give it another go.

[Zizaco]

@WyriHaximus Any news on this?

[WyriHaximus]

@Zizaco Honestly I have no reason not to do this unless @reactphp/core has objections. There are however a few things I'de like to see improved but I'm not sure if @barrylb can/wants to do those. Or that I pull it into a feature branch, fix those and PR it to master.

[clue]

@barrylb I'd love to get this feature in and appreciate your effort 👍 (see also #39)

This is kind of an old PR and it contains plenty of merge conflicts now. Are you still interested in updating this? (See also #76)

Afaict this feature depends on proper detection of message payloads first (empty bodies, chunked transfer encoding and content-length), see #104.

[clue]

As much as I'd love to get this feature in, I'm having to close this for now as it hasn't received any input in a while and it's unlikely this will get traction any time soon.

The feature request is still open in #39 and we'll look into this in the not too far future 👍 If you feel this was closed prematurely or want to pick this up again, please let us know and we can reopen this.

Thank you for your effort!