std(http/server): close open connections on server close #3679
Conversation
bartlomieju
changed the title
|
Okay after some more thought I think problem described in #3593 is caused by two separate issues:
|
bartlomieju
force-pushed
the
fix_hanging_http_server
branch
from
57f01dd to
29e27a0
Compare
|
|
bartlomieju
force-pushed
the
fix_hanging_http_server
branch
from
29e27a0 to
9ecfbeb
Compare
Let's see, just updated tests |
|
@bartlomieju Okay I've seen it now (this behaviour:
). It won't stop trying to read requests on a connection until that connection closes... which is entirely in the client's hands. This still isn't necessarily a bad thing since people can implement their own timeouts etc. -- but at the same time there will a natural expectation that the iterators break on server close. Probably the right call to drop connections then 👍. |
bartlomieju
changed the title
|
Just throwing this out there: an alternative solution would be to provide a separate |
Yes, we discussed that offline, and indeed @nayeemrmn if you'd be interested in implementing that then using Go's approach is encouraged: |
Due to complaints about ergonomics and because it breaks shebang on linux. This reverts commit 2d5457d. BREAKING CHANGE
|
Needs more investigation, closing for now |
|
With #4293 this PR can be moved forward |
|
@ry please review |
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
…no#3679) Due to structure of "Server" for each open connection there's a pending "read" op. Because connection owned by "Server" are not tracked, calling "Server.close()" doesn't close open connections. This commit introduces simple tracking of connections for server and ensures owned connections are closed on "Server.close()".
Fixes #3390
If you call
server.close()on a server that has pending connections then Deno process won't exit until next request comes over for each connection.This PR ensures that all open connections that server owns get properly closed when server is closed.