node
node copied to clipboard
http: fix keepAliveTimeout for HTTP server after answering a POST request synchronously
This is a draft PR for fixing #39137 ... primarily for the test-case for now.
My changes to lib/_http_server.js
are just an experiment as a base for discussion - it fixes this issue, but I don't want to fix it that way (and it currently breaks other tests):
- I don't want a new
responseSent
flag - I'm sure we can use an existing flag, but I didn't find a suitable one - Maybe the
resetSocketTimeout()
inonParserExecuteCommon()
can be even moved to somewhere else? Not when data (body?) arrives, but when the request (header?) arrives - It seems
resetSocketTimeout()
only resets the timeout ifkeepAliveTimeoutSet
, which is set only inresOnFinish
... so maybe the experimentalresponseSent
flag makeskeepAliveTimeoutSet
useless - Resetting
responseSent
inonParserExecuteCommon()
is probably the wrong place anyway ... currently theese two tests are broken:- sequential/test-http-server-keep-alive-timeout-slow-client-headers.js
- sequential/test-http-server-keep-alive-timeout-slow-server.js
@ronag how do you think we can unstuck this PR?
@indutny could you take a look at the top issue https://github.com/nodejs/node/issues/39137? Maybe you have an implementation hint.
This part of the code is quite tricky. I would have to dig into it.