feat: unref socket while idle #772
Conversation
Allow process shutdown once all requests have completed without waiting for keepAliveTimeout.
|
Needs test. |
|
@Ethan-Arrowood ping regarding your question on Slack. If you want to help land please look at making a test. |
Codecov Report
@@ Coverage Diff @@
## main #772 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 24 24
Lines 1904 2488 +584
==========================================
+ Hits 1904 2488 +584
Continue to review full report at Codecov.
|
|
Added test which unfortunately fails. @mcollina do you know how to debug/check what root ref keeps the process alive? |
|
Maybe @jasnell has some tip on how to debug what ref is keeping the event loop alive? |
|
This looks good! I'll try and add some more test cases later today. |
|
have you tried https://github.com/mafintosh/why-is-node-running? |
|
Yea, so the code works. The problem is the test. I think the server and client needs to run in a separate process for it to work. FYI @Ethan-Arrowood |
|
I'm trying a multi-process test now using worker threads. will report back shortly with what I come up with I sorta have it working, but not really /testing/ anything. Gotta figure out how to make tap work correctly in a worker thread scenario |
|
This is what I have so far: https://gist.github.com/Ethan-Arrowood/30341384a724f944a334b2cae7e4f739 Details in the gist title. Still hacking at it to see if I can get tap to play nicely. |
|
I think you should put the server into the worker, not the other way around. |
|
worker_thread test PR is up against this branch (from my fork) |
|
irregardless of the worker thread test. The existing connect-timeout test is failing with this new change. Any ideas? Do we need to keep the client referenced while the making progress. its not the client, its the underlying socket. since this test dereferences the socket in the net.connect override, the client closes itself before processing the request. |
* fix unref test to use worker threads * fix import path
|
@Ethan-Arrowood I think I managed to fix it. |
ronag
force-pushed
the
unref-when-idle
branch
2 times, most recently
from
b8a7115 to
0cf1e47
Compare
|
Which release will this be published in? 3 or 4? |
|
4 |
* feat: unref socket while idle Allow process shutdown once all requests have completed without waiting for keepAliveTimeout. * fixup * fixup * fixup * fixup * fixup * fixup * fixup * fixup * fixup * fixup * fixup * fix unref test to use worker threads (nodejs#775) * fix unref test to use worker threads * fix import path * fixup: connect test * fixup * fixup: jest Co-authored-by: Ethan Arrowood <ethan.arrowood@gmail.com>
Allow process shutdown once all requests have completed without
waiting for keepAliveTimeout.