http: fix http server connection list when close by theanarkh · Pull Request #43949 · nodejs/node · GitHub

Refs: Failed CI
Refs: Failed CI
Refs: #43638 (comment)

When the socket close, the related parser should be removed out of the server connection list.

When the socket close, the related parser will be cleared(_http_server.js:socketOnClose -> freeParser), the related code of freeParser is as follows.

cleanParser(parser);
if (parsers.free(parser) === false) {
    setImmediate(closeParserInstance, parser);
} else {
    parser.free();
}

cleanParser(parser) make the socket.parser is null, and if parsers.free(parser) is true anything is ok because it will remove the parser out of server.connectionList. But if it is false it will trigger bugs.

If the user listen to close event of socket and call server.close in the callback of close event, the server.close will call closeIdleConnections to close all idle sockets, the code is as follows.

Server.prototype.closeIdleConnections = function() {
    const connections = this[kConnections].idle();
    for (let i = 0, l = connections.length; i < l; i++) {
        if (connections[i].socket._httpMessage && !connections[i].socket._httpMessage.finished) {
          continue;
        }
    }
};

connections[i] is a parser object, and parser.socket is null which make the bug(Cannot read properties of null (reading '_httpMessage')).

If user call the code as follows in callback of close event

setImmediate(() => {
    server.close();
});

it will make the process crashed because when closeParserInstance in setImmediate(closeParserInstance, parser); is called, it will delete the C++ parser object. Then Node.js will call the second callback(() => { server.close(); }) of setImmediate, server.close() will call this[kConnections].idle() which will access C++ layer to get the parser list, but the parser is delete so make the process crashed. The codedump is as follows.

    frame #0: 0x0000000101cf0498 node`v8::internal::JSObject::AddDataElement(v8::internal::Handle<v8::internal::JSObject>, unsigned int, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes) [inlined] v8::internal::Handle<v8::internal::Object>::operator*(this=<unavailable>) const at handles.h:141:37 [opt]
    frame #1: 0x0000000101cf0498 node`v8::internal::JSObject::AddDataElement(v8::internal::Handle<v8::internal::JSObject>, unsigned int, v8::internal::Handle<v8::internal::Object>, v8::internal::PropertyAttributes) [inlined] v8::internal::Handle<v8::internal::Object>::operator->(this=<unavailable>) const at handles.h:135 [opt]
    frame #2: 0x0000000101cf0498 node`v8::internal::JSObject::AddDataElement(object=Handle<v8::internal::JSObject> @ 0x00007ffeee585a28, index=<unavailable>, value=<unavailable>, attributes=NONE) at js-objects.cc:5194 [opt]
    frame #3: 0x0000000101d535f2 node`v8::internal::Object::AddDataProperty(it=0x00007ffeee585ae0, value=<unavailable>, attributes=<unavailable>, should_throw=(has_value_ = true, value_ = kDontThrow), store_origin=<unavailable>, semantics=<unavailable>) at objects.cc:2914:5 [opt]
    frame #4: 0x00000001018c5efc node`v8::Object::Set(v8::Local<v8::Context>, unsigned int, v8::Local<v8::Value>) [inlined] v8::internal::Object::SetElement(isolate=<unavailable>, object=Handle<v8::internal::Object> @ r15, index=<unavailable>, value=Handle<v8::internal::Object> @ r14, should_throw=kDontThrow) at objects-inl.h:657:3 [opt]
    frame #5: 0x00000001018c5ec1 node`v8::Object::Set(this=0x00007fb667814658, context=<unavailable>, index=<unavailable>, value=(val_ = 0x0000000000000000)) at api.cc:4282 [opt]
    frame #6: 0x000000010176499c node`node::(anonymous namespace)::ConnectionsList::Idle(args=0x00007ffeee585c50) at node_http_parser.cc:1065:17 [opt]
    frame #7: 0x000000010191c228 node`v8::internal::FunctionCallbackArguments::Call(this=0x00007ffeee585ca8, handler=<unavailable>) at api-arguments-inl.h:147:3 [opt]
    frame #8: 0x000000010191bd42 node`v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(isolate=0x00007fb66752b000, function=<unavailable>, new_target=<unavailable>, fun_data=Handle<v8::internal::FunctionTemplateInfo> @ r13, receiver=<unavailable>, args=BuiltinArguments @ 0x00007ffeee585d50) at builtins-api.cc:112:36 [opt]
    frame #9: 0x000000010191b38f node`v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) at builtins-api.cc:142:5 [opt]
    frame #10: 0x000000010191b2aa node`v8::internal::Builtin_HandleApiCall(args_length=<unavailable>, args_object=0x00007ffeee585e00, isolate=0x00007fb66752b000) at builtins-api.cc:130 [opt]
    frame #11: 0x000000010225d479 node`Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit + 57
    frame #12: 0x00000001021e13d0 node`Builtins_InterpreterEntryTrampoline + 208
    frame #13: 0x00000001021e13d0 node`Builtins_InterpreterEntryTrampoline + 208
    frame #14: 0x00000001021e13d0 node`Builtins_InterpreterEntryTrampoline + 208
    frame #15: 0x00000001021e13d0 node`Builtins_InterpreterEntryTrampoline + 208
    frame #16: 0x00000001021df9dc node`Builtins_JSEntryTrampoline + 92
    frame #17: 0x00000001021df703 node`Builtins_JSEntry + 131
    frame #18: 0x00000001019e68bc node`v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [inlined] v8::internal::GeneratedCode<unsigned long, unsigned long, unsigned long, unsigned long, unsigned long, long, unsigned long**>::Call(this=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>, args=<unavailable>) at simulator.h:156:12 [opt]
    frame #19: 0x00000001019e68b9 node`v8::internal::(anonymous namespace)::Invoke(isolate=0x00007fb66752b000, params=0x00007ffeee586190)::InvokeParams const&) at execution.cc:425 [opt]
    frame #20: 0x00000001019e5ca5 node`v8::internal::Execution::Call(isolate=0x00007fb66752b000, callable=<unavailable>, receiver=<unavailable>, argc=0, argv=0x0000000000000000) at execution.cc:523:10 [opt]
    frame #21: 0x00000001018cc5f9 node`v8::Function::Call(this=0x00007fb66783a520, context=<unavailable>, recv=<unavailable>, argc=<unavailable>, argv=0x0000000000000000) at api.cc:5255:7 [opt]
    frame #22: 0x000000010167af2f node`node::InternalMakeCallback(env=0x00007fb66814e600, resource=(val_ = 0x00007fb66783a600), recv=<unavailable>, callback=<unavailable>, argc=0, argv=0x0000000000000000, asyncContext=(async_id = 0, trigger_async_id = 0)) at callback.cc:211:21 [opt]
    frame #23: 0x000000010167b33c node`node::MakeCallback(isolate=0x00007fb66752b000, recv=(val_ = 0x00007fb66783a600), callback=(val_ = 0x00007fb66783a520), argc=0, argv=0x0000000000000000, asyncContext=(async_id = 0, trigger_async_id = 0)) at callback.cc:282:7 [opt]
    frame #24: 0x00000001016e8645 node`node::Environment::CheckImmediate(handle=<unavailable>) at env.cc:1331:5 [opt]
    frame #25: 0x00000001021c87c4 node`uv__run_check(loop=0x00000001053cb630) at loop-watcher.c:67:1 [opt]
    frame #26: 0x00000001021c1d31 node`uv_run(loop=0x00000001053cb630, mode=UV_RUN_DEFAULT) at core.c:398:5 [opt]
    frame #27: 0x000000010167b743 node`node::SpinEventLoop(env=0x00007fb66814e600) at embed_helpers.cc:37:7 [opt]
    frame #28: 0x00000001017879e1 node`node::NodeMainInstance::Run(this=<unavailable>, exit_code=0x00007ffeee5866fc, env=0x00007fb66814e600) at node_main_instance.cc:140:18 [opt]
    frame #29: 0x0000000101787653 node`node::NodeMainInstance::Run(this=<unavailable>) at node_main_instance.cc:132:3 [opt]
    frame #30: 0x00000001017170c6 node`node::Start(argc=<unavailable>, argv=<unavailable>) at node.cc:1199:38 [opt]
    frame #31: 0x00007fff205fe621 libdyld.dylib`start + 1
    frame #32: 0x00007fff205fe621 libdyld.dylib`start + 1

cc @ShogunPanda

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Review requested:

@nodejs/http
@nodejs/net

lgtm

LGTM!

Thanks for taking care of this!

CI: https://ci.nodejs.org/job/node-test-pull-request/45608/

Let's fast-track this.

cc @tniessen

Fast-track has been requested by @mcollina. Please 👍 to approve.

Fast-track has been requested by @F3n67u. Please 👍 to approve.

Sorry that I remove fast-track label wrongly. add it again.

node-stress-single-test comparison for sequential/test-gc-http-client-timeout

This pr (No Failure)

https://ci.nodejs.org/view/Stress/job/node-stress-single-test/359/

Main Branch (Failure)

https://ci.nodejs.org/job/node-stress-single-test/nodes=ubuntu2004-arm64/357/

Don't know why the error is crash but not Cannot read properties of null

Error Message
crashed (-11)
Stacktrace
done/collected/total: 157/0/804
done/collected/total: 322/157/804
done/collected/total: 479/322/804
done/collected/total: 801/413/804
done/collected/total: 801/479/804
done/collected/total: 801/801/804
done/collected/total: 804/801/804
done/collected/total: 804/801/804
done/collected/total: 804/804/804

Trigger another stress test for sequential/test-gc-http-client & sequential/test-gc-http-client-onerror to verfiy if those flaky test is fixed: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/360/

node-stress-single-test comparison for test-gc-http-client & test-gc-http-client-onerror

This pr (No Failure)

https://ci.nodejs.org/view/Stress/job/node-stress-single-test/360/

Main Branch (Failure)

https://ci.nodejs.org/view/Stress/job/node-stress-single-test/361/

node-stress-single-test comparison for sequential/test-gc-http-client-timeout

This pr (No Failure)

https://ci.nodejs.org/view/Stress/job/node-stress-single-test/359/

Main Branch (Failure)

https://ci.nodejs.org/job/node-stress-single-test/nodes=ubuntu2004-arm64/357/

Don't know why the error is crash but not Cannot read properties of null
Error Message
crashed (-11)
Stacktrace
done/collected/total: 157/0/804
done/collected/total: 322/157/804
done/collected/total: 479/322/804
done/collected/total: 801/413/804
done/collected/total: 801/479/804
done/collected/total: 801/801/804
done/collected/total: 804/801/804
done/collected/total: 804/801/804
done/collected/total: 804/804/804

Thanks for testing this. Cannot read properties of null and crash is two bugs this pr try to fix. If there are too many requests it will often trigger crash.

Commit Queue failed

- Loading data for nodejs/node/pull/43949
✔  Done loading data for nodejs/node/pull/43949
----------------------------------- PR info ------------------------------------
Title      http: fix http server connection list when close (#43949)
Author     theanarkh  (@theanarkh)
Branch     theanarkh:fix_server_connection_list -> nodejs:main
Labels     http, c++, http_parser, fast-track, author ready
Commits    1
 - http: fix http server connection list when close
Committers 1
 - theanarkh 
PR-URL: https://github.com/nodejs/node/pull/43949
Reviewed-By: Matteo Collina 
Reviewed-By: Paolo Insogna 
Reviewed-By: Feng Yu 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43949
Reviewed-By: Matteo Collina 
Reviewed-By: Paolo Insogna 
Reviewed-By: Feng Yu 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 22 Jul 2022 18:59:11 GMT
   ✔  Approvals: 3
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/43949#pullrequestreview-1048331522
   ✔  - Paolo Insogna (@ShogunPanda): https://github.com/nodejs/node/pull/43949#pullrequestreview-1048515689
   ✔  - Feng Yu (@F3n67u): https://github.com/nodejs/node/pull/43949#pullrequestreview-1048600770
   ℹ  This PR is being fast-tracked
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-07-24T09:13:51Z: https://ci.nodejs.org/job/node-test-pull-request/45629/
- Querying data for job/node-test-pull-request/45629/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2727070300

Landed in da3949c

PR-URL: #43949 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Feng Yu <F3n67u@outlook.com>

PR-URL: #43971 Refs: #43949 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

Should this be backported? parser->connectionsList_ doesn't exist in v16.x

If parser->connectionsList_ doesn't exist in v16.x, i think we can not backport this to v16.x.

##### [\`v18.7.0\`](https://github.com/nodejs/node/releases/tag/v18.7.0) ##### Notable changes - **doc**: - add F3n67u to collaborators (Feng Yu) [#43953](nodejs/node#43953) - deprecate coercion to integer in process.exit (Daeyeon Jeong) [#43738](nodejs/node#43738) - **(SEMVER-MINOR)** deprecate diagnostics_channel object subscribe method (Stephen Belanger) [#42714](nodejs/node#42714) - **events**: - **(SEMVER-MINOR)** expose CustomEvent on global with CLI flag (Daeyeon Jeong) [#43885](nodejs/node#43885) - **(SEMVER-MINOR)** add `CustomEvent` (Daeyeon Jeong) [#43514](nodejs/node#43514) - **http**: - **(SEMVER-MINOR)** add drop request event for http server (theanarkh) [#43806](nodejs/node#43806) - **lib**: - **(SEMVER-MINOR)** improved diagnostics_channel subscribe/unsubscribe (Stephen Belanger) [#42714](nodejs/node#42714) - **util**: - **(SEMVER-MINOR)** add tokens to parseArgs (John Gee) [#43459](nodejs/node#43459) ##### Commits - \[[`0aa255ab72`](nodejs/node@0aa255ab72)] - **bootstrap**: handle snapshot errors gracefully (Joyee Cheung) [#43531](nodejs/node#43531) - \[[`0783ddf57e`](nodejs/node@0783ddf57e)] - **buffer**: do not leak memory if buffer is too big (Keyhan Vakil) [#43938](nodejs/node#43938) - \[[`12657accdd`](nodejs/node@12657accdd)] - **build**: add .gitattributes for npm and other shims (Hrishikesh Kadam) [#43879](nodejs/node#43879) - \[[`c2db4f4581`](nodejs/node@c2db4f4581)] - **build**: make GitPod less noisy (Rich Trott) [#43829](nodejs/node#43829) - \[[`364deeadcd`](nodejs/node@364deeadcd)] - **build**: add GitHub token permissions for workflows (Varun Sharma) [#43743](nodejs/node#43743) - \[[`8b83b4d5be`](nodejs/node@8b83b4d5be)] - **child_process**: do not need to count length when maxBuffer is Infinity (theanarkh) [#43822](nodejs/node#43822) - \[[`c1893b7a7c`](nodejs/node@c1893b7a7c)] - **child_process**: avoid repeated calls to `normalizeSpawnArguments` (木杉) [#43345](nodejs/node#43345) - \[[`7b276b89b9`](nodejs/node@7b276b89b9)] - **cluster**: send connection to other server when worker drop it (theanarkh) [#43747](nodejs/node#43747) - \[[`e8c66f92a5`](nodejs/node@e8c66f92a5)] - **crypto**: remove unneeded guard (Rich Trott) [#43856](nodejs/node#43856) - \[[`c95132e9ea`](nodejs/node@c95132e9ea)] - **deps**: cherry-pick [libuv/libuv@`3a7b955`](libuv/libuv@3a7b955) (Ben Noordhuis) [#43950](nodejs/node#43950) - \[[`cc8d5426d2`](nodejs/node@cc8d5426d2)] - **deps**: cherry-pick [libuv/libuv@`abb109f`](libuv/libuv@abb109f) (Ben Noordhuis) [#43950](nodejs/node#43950) - \[[`7762e463d6`](nodejs/node@7762e463d6)] - **deps**: update corepack to 0.12.1 (Node.js GitHub Bot) [#43965](nodejs/node#43965) - \[[`1256c4dad5`](nodejs/node@1256c4dad5)] - **deps**: update hast-util-raw (Moshe Atlow) [#43927](nodejs/node#43927) - \[[`aac97c2d2a`](nodejs/node@aac97c2d2a)] - **deps**: update undici to 5.8.0 (Node.js GitHub Bot) [#43886](nodejs/node#43886) - \[[`cdff61917d`](nodejs/node@cdff61917d)] - **deps**: clean archs files for OpenSSL (RafaelGSS) [#43735](nodejs/node#43735) - \[[`fc936a84e4`](nodejs/node@fc936a84e4)] - **deps**: remove not used architectures (RafaelGSS) [#43735](nodejs/node#43735) - \[[`361a643d8b`](nodejs/node@361a643d8b)] - **deps**: V8: backport [`f3cad8c`](nodejs/node@f3cad8cec656) (Joyee Cheung) [#43531](nodejs/node#43531) - \[[`2e1732ebd0`](nodejs/node@2e1732ebd0)] - **deps**: V8: backport [`22698d2`](nodejs/node@22698d267667) (Chengzhong Wu) [#43751](nodejs/node#43751) - \[[`979f469d3a`](nodejs/node@979f469d3a)] - **deps**: upgrade npm to 8.15.0 (npm team) [#43917](nodejs/node#43917) - \[[`4096d81988`](nodejs/node@4096d81988)] - **deps**: upgrade npm to 8.14.0 (npm team) [#43826](nodejs/node#43826) - \[[`2ec8092e2c`](nodejs/node@2ec8092e2c)] - **deps,src**: use SIMD for normal base64 encoding (Brian White) [#39775](nodejs/node#39775) - \[[`67b4edde37`](nodejs/node@67b4edde37)] - **dns**: fix getServers return undefined (jiahao.si) [#43922](nodejs/node#43922) - \[[`7c75539a88`](nodejs/node@7c75539a88)] - **dns**: fix cares memory leak (theanarkh) [#43912](nodejs/node#43912) - \[[`1f80b88da5`](nodejs/node@1f80b88da5)] - **doc**: update email and mailmap for BethGriggs (Beth Griggs) [#43985](nodejs/node#43985) - \[[`8a2a6e16eb`](nodejs/node@8a2a6e16eb)] - **doc**: add 15.x - 18.x to Other Versions section (shhh7612) [#43940](nodejs/node#43940) - \[[`51cb0d42ca`](nodejs/node@51cb0d42ca)] - **doc**: inspector.close undefined in worker threads (Keyhan Vakil) [#43867](nodejs/node#43867) - \[[`c789c0f5f7`](nodejs/node@c789c0f5f7)] - **doc**: improve documentation for safe `Promise` statics alternatives (Antoine du Hamel) [#43759](nodejs/node#43759) - \[[`cb9b0e0011`](nodejs/node@cb9b0e0011)] - **doc**: recommend git-node-v8 (Keyhan Vakil) [#43934](nodejs/node#43934) - \[[`d7e9bd1830`](nodejs/node@d7e9bd1830)] - **doc**: clarify subprocess.stdout/in/err property (Kohei Ueno) [#43910](nodejs/node#43910) - \[[`808793ebb5`](nodejs/node@808793ebb5)] - **doc**: fix typo in `src/crypto/README.md` (Jianru Lin) [#43968](nodejs/node#43968) - \[[`bbc455c4f9`](nodejs/node@bbc455c4f9)] - **doc**: remind backporter about v8\_embedder_string (Keyhan Vakil) [#43924](nodejs/node#43924) - \[[`a86b66c8b4`](nodejs/node@a86b66c8b4)] - **doc**: fix typo in http.md (Airing) [#43933](nodejs/node#43933) - \[[`a96af37233`](nodejs/node@a96af37233)] - **doc**: add F3n67u to collaborators (Feng Yu) [#43953](nodejs/node#43953) - \[[`aa7d4e59f7`](nodejs/node@aa7d4e59f7)] - **doc**: improve test runner timeout docs (Tobias Nießen) [#43836](nodejs/node#43836) - \[[`80c2fa8212`](nodejs/node@80c2fa8212)] - **doc**: mention Win 32-bit openssl build issue (RafaelGSS) [#43853](nodejs/node#43853) - \[[`8b8c55df7e`](nodejs/node@8b8c55df7e)] - **doc**: add security release specifics to releases.md (Beth Griggs) [#43835](nodejs/node#43835) - \[[`42693aaf9f`](nodejs/node@42693aaf9f)] - **doc**: add history info for `global.performance` (Antoine du Hamel) [#43841](nodejs/node#43841) - \[[`140d6af572`](nodejs/node@140d6af572)] - **doc**: add platform-windows-arm to who to CC (Michael Dawson) [#43808](nodejs/node#43808) - \[[`976093efe3`](nodejs/node@976093efe3)] - **doc**: document ES2022's Error "cause" property (James Ide) [#43830](nodejs/node#43830) - \[[`ec7e45e4a2`](nodejs/node@ec7e45e4a2)] - **doc**: include make clean to openssl arch (RafaelGSS) [#43735](nodejs/node#43735) - \[[`d64dfd53c9`](nodejs/node@d64dfd53c9)] - **doc**: add link to diagnostic tools (Rafael Gonzaga) [#43736](nodejs/node#43736) - \[[`2910136920`](nodejs/node@2910136920)] - **doc**: update links to MDN page about dynamic imports (Jannis R) [#43847](nodejs/node#43847) - \[[`d88a9fae79`](nodejs/node@d88a9fae79)] - **doc**: deprecate coercion to integer in process.exit (Daeyeon Jeong) [#43738](nodejs/node#43738) - \[[`fc843e103d`](nodejs/node@fc843e103d)] - **doc**: add MoLow to triagers (Moshe Atlow) [#43799](nodejs/node#43799) - \[[`8c8c97da61`](nodejs/node@8c8c97da61)] - **(SEMVER-MINOR)** **doc**: deprecate diagnostics_channel object subscribe method (Stephen Belanger) [#42714](nodejs/node#42714) - \[[`9b53a694b5`](nodejs/node@9b53a694b5)] - **doc**: revert anachronistic 'node:' module require()s in API history notes (DeeDeeG) [#43768](nodejs/node#43768) - \[[`2815bd3002`](nodejs/node@2815bd3002)] - **doc**: clarify release process for new releasers (Rafael Gonzaga) [#43739](nodejs/node#43739) - \[[`50b3750e67`](nodejs/node@50b3750e67)] - **doc**: fix typo in ngtcp2 readme (Dan Castillo) [#43767](nodejs/node#43767) - \[[`6bcd40dd85`](nodejs/node@6bcd40dd85)] - **domain**: fix vm promise tracking while keeping isolation (Stephen Belanger) [#43556](nodejs/node#43556) - \[[`e89e0b470b`](nodejs/node@e89e0b470b)] - **esm**: remove superfluous argument (Rich Trott) [#43884](nodejs/node#43884) - \[[`0d2921f396`](nodejs/node@0d2921f396)] - **esm**: fix erroneous re-initialization of ESMLoader (Jacob Smith) [#43763](nodejs/node#43763) - \[[`9b5b8d78c3`](nodejs/node@9b5b8d78c3)] - **esm**: throw on any non-2xx response (LiviaMedeiros) [#43742](nodejs/node#43742) - \[[`dfc4832ef1`](nodejs/node@dfc4832ef1)] - **(SEMVER-MINOR)** **events**: expose CustomEvent on global with CLI flag (Daeyeon Jeong) [#43885](nodejs/node#43885) - \[[`e4473952ae`](nodejs/node@e4473952ae)] - **(SEMVER-MINOR)** **events**: add `CustomEvent` (Daeyeon Jeong) [#43514](nodejs/node#43514) - \[[`100f6deb09`](nodejs/node@100f6deb09)] - **fs**: use signed types for stat data (LiviaMedeiros) [#43714](nodejs/node#43714) - \[[`25ec71db63`](nodejs/node@25ec71db63)] - **http**: fix http server connection list when close (theanarkh) [#43949](nodejs/node#43949) - \[[`ca658c8afe`](nodejs/node@ca658c8afe)] - **(SEMVER-MINOR)** **http**: add drop request event for http server (theanarkh) [#43806](nodejs/node#43806) - \[[`9c699bd8a8`](nodejs/node@9c699bd8a8)] - **http**: wait for pending responses in closeIdleConnections (Paolo Insogna) [#43890](nodejs/node#43890) - \[[`781d5e54e3`](nodejs/node@781d5e54e3)] - **inspector**: set sampling interval before start (Shelley Vohr) [#43779](nodejs/node#43779) - \[[`0b5dbb2a56`](nodejs/node@0b5dbb2a56)] - **lib**: refactor PriorityQueue to use private field (Finn Yu) [#43889](nodejs/node#43889) - \[[`324473ca32`](nodejs/node@324473ca32)] - **(SEMVER-MINOR)** **lib**: improved diagnostics_channel subscribe/unsubscribe (Stephen Belanger) [#42714](nodejs/node#42714) - \[[`5aa3b213ac`](nodejs/node@5aa3b213ac)] - **meta**: update AUTHORS (Node.js GitHub Bot) [#43966](nodejs/node#43966) - \[[`e707552357`](nodejs/node@e707552357)] - **meta**: update `node-api` in label-pr-config (Daeyeon Jeong) [#43794](nodejs/node#43794) - \[[`8a8de94034`](nodejs/node@8a8de94034)] - **meta**: update AUTHORS (Node.js GitHub Bot) [#43872](nodejs/node#43872) - \[[`7d49fc766c`](nodejs/node@7d49fc766c)] - **meta**: use platform dropdown on flaky template (Rafael Gonzaga) [#43855](nodejs/node#43855) - \[[`e4aa50fc3f`](nodejs/node@e4aa50fc3f)] - **meta**: enable blank issues (Matteo Collina) [#43775](nodejs/node#43775) - \[[`ceb7c150ec`](nodejs/node@ceb7c150ec)] - **meta**: move one or more collaborators to emeritus (Node.js GitHub Bot) [#43770](nodejs/node#43770) - \[[`29bcd47738`](nodejs/node@29bcd47738)] - **net**: fix socket.\_getpeername (Daeyeon Jeong) [#43010](nodejs/node#43010) - \[[`380659daf1`](nodejs/node@380659daf1)] - **process**: use `defineProperty` instead of assignment (Mark S. Miller) [#43907](nodejs/node#43907) - \[[`aba9c8ebea`](nodejs/node@aba9c8ebea)] - **repl**: fix overzealous top-level await (Tobias Nießen) [#43827](nodejs/node#43827) - \[[`1deb6b73b7`](nodejs/node@1deb6b73b7)] - **repl**: use `SafePromiseAll` and `SafePromiseRace` (Antoine du Hamel) [#43758](nodejs/node#43758) - \[[`bf8f2e23ff`](nodejs/node@bf8f2e23ff)] - **src**: refactor DH groups to delete crypto_groups.h (Tobias Nießen) [#43896](nodejs/node#43896) - \[[`9435fbf8cd`](nodejs/node@9435fbf8cd)] - **src**: remove dead code in base64\_encode (Tobias Nießen) [#43979](nodejs/node#43979) - \[[`2c47e58ea0`](nodejs/node@2c47e58ea0)] - **src**: fix regression that a source marker is lost (cola119) [#43086](nodejs/node#43086) - \[[`d084150320`](nodejs/node@d084150320)] - **src**: per-isolate eternal template properties (Chengzhong Wu) [#43802](nodejs/node#43802) - \[[`9f9d00ccbb`](nodejs/node@9f9d00ccbb)] - **src**: merge NativeModuleEnv into NativeModuleLoader (Joyee Cheung) [#43824](nodejs/node#43824) - \[[`bb512904e9`](nodejs/node@bb512904e9)] - **src**: use named struct instead of typedef (Tobias Nießen) [#43881](nodejs/node#43881) - \[[`bb5511e8cc`](nodejs/node@bb5511e8cc)] - **src**: use named enum instead of typedef (Tobias Nießen) [#43880](nodejs/node#43880) - \[[`5db0c8f667`](nodejs/node@5db0c8f667)] - **src**: pass only Isolate\* and env_vars to EnabledDebugList::Parse() (Darshan Sen) [#43668](nodejs/node#43668) - \[[`249365524e`](nodejs/node@249365524e)] - **src**: fix node watchdog race condition (theanarkh) [#43780](nodejs/node#43780) - \[[`17cb27237d`](nodejs/node@17cb27237d)] - **src**: deduplicate `SetALPN` implementations (Tobias Nießen) [#43756](nodejs/node#43756) - \[[`b4c75a96be`](nodejs/node@b4c75a96be)] - **src**: fix `napi_check_object_type_tag()` (Daeyeon Jeong) [#43788](nodejs/node#43788) - \[[`8432d6596f`](nodejs/node@8432d6596f)] - **src**: slim down env-inl.h (Ben Noordhuis) [#43745](nodejs/node#43745) - \[[`2266a4b6d6`](nodejs/node@2266a4b6d6)] - **stream**: improve `respondWithNewView()` (Daeyeon Jeong) [#43866](nodejs/node#43866) - \[[`bf3991b406`](nodejs/node@bf3991b406)] - **stream**: fix 0 transform hwm backpressure (Robert Nagy) [#43685](nodejs/node#43685) - \[[`a057510037`](nodejs/node@a057510037)] - **stream**: initial approach to include strategy options on Readable.toWeb() (txxnano) [#43515](nodejs/node#43515) - \[[`198cf59d2c`](nodejs/node@198cf59d2c)] - **test**: update WPT encoding tests (Kohei Ueno) [#43958](nodejs/node#43958) - \[[`f0ed1aed8d`](nodejs/node@f0ed1aed8d)] - **test**: remove test-whatwg-events-add-event-listener-options-once.js (Feng Yu) [#43877](nodejs/node#43877) - \[[`88505556fe`](nodejs/node@88505556fe)] - **test**: work scheduled in process.nextTick can keep the event loop alive (Andreu Botella) [#43787](nodejs/node#43787) - \[[`81a21946eb`](nodejs/node@81a21946eb)] - **test**: simplify test-tls-set-secure-context (Tobias Nießen) [#43878](nodejs/node#43878) - \[[`61cd11a8a7`](nodejs/node@61cd11a8a7)] - **test**: use `common.mustNotMutateObjectDeep()` in fs tests (LiviaMedeiros) [#43819](nodejs/node#43819) - \[[`b1081dbe12`](nodejs/node@b1081dbe12)] - **test**: fix test http upload timeout (theanarkh) [#43935](nodejs/node#43935) - \[[`efd5e0e925`](nodejs/node@efd5e0e925)] - **test**: simplify ReplStream.wait() (Tobias Nießen) [#43857](nodejs/node#43857) - \[[`ef21ad2996`](nodejs/node@ef21ad2996)] - **test**: merge test-crypto-dh-hash with modp18 test (Tobias Nießen) [#43891](nodejs/node#43891) - \[[`e502c50a90`](nodejs/node@e502c50a90)] - **test**: refactor `test/es-module/test-esm-resolve-type` (Antoine du Hamel) [#43178](nodejs/node#43178) - \[[`c782c3dc69`](nodejs/node@c782c3dc69)] - **test**: ensure NODE_EXTRA_CA_CERTS not set before tests (KrayzeeKev) [#43858](nodejs/node#43858) - \[[`bb6787cb57`](nodejs/node@bb6787cb57)] - **test**: add check to test-fs-readfile-tostring-fail (Richard Lau) [#43850](nodejs/node#43850) - \[[`7571704186`](nodejs/node@7571704186)] - **test**: complete TODO in test/wpt/test-url.js (Kohei Ueno) [#43797](nodejs/node#43797) - \[[`6f1d2dfb9d`](nodejs/node@6f1d2dfb9d)] - **test**: add test on worker process.exit in async modules (Chengzhong Wu) [#43751](nodejs/node#43751) - \[[`776cc3abbd`](nodejs/node@776cc3abbd)] - **test**: use `common.mustNotMutateObjectDeep()` in immutability tests (LiviaMedeiros) [#43196](nodejs/node#43196) - \[[`42f2deb3a0`](nodejs/node@42f2deb3a0)] - **test**: add `common.mustNotMutateObjectDeep()` (LiviaMedeiros) [#43196](nodejs/node#43196) - \[[`f3fc51c508`](nodejs/node@f3fc51c508)] - **test**: fix coverity warning in test (Michael Dawson) [#43631](nodejs/node#43631) - \[[`a9ecba2fa8`](nodejs/node@a9ecba2fa8)] - **test**: mark test-http-client-response-timeout flaky (Tobias Nießen) [#43792](nodejs/node#43792) - \[[`cd0d9ddb7c`](nodejs/node@cd0d9ddb7c)] - **test_runner**: add support for boolean values for `concurrency` option (Lenvin Gonsalves) [#43887](nodejs/node#43887) - \[[`f98020138a`](nodejs/node@f98020138a)] - **test_runner**: validate `timeout` option (Antoine du Hamel) [#43843](nodejs/node#43843) - \[[`58d15b3687`](nodejs/node@58d15b3687)] - **test_runner**: pass signal on timeout (Moshe Atlow) [#43911](nodejs/node#43911) - \[[`8b0248506f`](nodejs/node@8b0248506f)] - **test_runner**: do not report an error when tests are passing (Antoine du Hamel) [#43919](nodejs/node#43919) - \[[`aa8053e1fa`](nodejs/node@aa8053e1fa)] - **test_runner**: recieve and pass AbortSignal (Moshe Atlow) [#43554](nodejs/node#43554) - \[[`f13e4c1be9`](nodejs/node@f13e4c1be9)] - **test_runner**: fix `it` concurrency (Moshe Atlow) [#43757](nodejs/node#43757) - \[[`e404a3ef6d`](nodejs/node@e404a3ef6d)] - **test_runner**: support timeout for tests (Moshe Atlow) [#43505](nodejs/node#43505) - \[[`f28198cc05`](nodejs/node@f28198cc05)] - **test_runner**: catch errors thrown within `describe` (Moshe Atlow) [#43729](nodejs/node#43729) - \[[`bfe0ac6cd0`](nodejs/node@bfe0ac6cd0)] - **tools**: add more options to track flaky tests (Antoine du Hamel) [#43954](nodejs/node#43954) - \[[`17a4e5e775`](nodejs/node@17a4e5e775)] - **tools**: add verbose flag to inactive TSC finder (Rich Trott) [#43913](nodejs/node#43913) - \[[`373304b0c7`](nodejs/node@373304b0c7)] - **tools**: add support for using API key to vuln checking script (Facundo Tuesca) [#43909](nodejs/node#43909) - \[[`ed45088c14`](nodejs/node@ed45088c14)] - **tools**: support versioned node shared libs on z/OS (alexcfyung) [#42256](nodejs/node#42256) - \[[`c9ecd6d21f`](nodejs/node@c9ecd6d21f)] - **tools**: update doc to highlight.js@11.6.0 (Node.js GitHub Bot) [#43870](nodejs/node#43870) - \[[`c92135aa0f`](nodejs/node@c92135aa0f)] - **tools**: update lint-md-dependencies to rollup@2.77.0 (Node.js GitHub Bot) [#43871](nodejs/node#43871) - \[[`e12bf40fd1`](nodejs/node@e12bf40fd1)] - **tools**: update eslint to 8.20.0 (Node.js GitHub Bot) [#43873](nodejs/node#43873) - \[[`09fe9b30a9`](nodejs/node@09fe9b30a9)] - **tools**: add script for vulnerability checking (Facundo Tuesca) [#43362](nodejs/node#43362) - \[[`19e8876877`](nodejs/node@19e8876877)] - **trace_events**: trace net connect event (theanarkh) [#43903](nodejs/node#43903) - \[[`1af7f24143`](nodejs/node@1af7f24143)] - **util**: remove unicode support todo for perf implications (Rhys) [#43762](nodejs/node#43762) - \[[`acfc33ca8c`](nodejs/node@acfc33ca8c)] - **(SEMVER-MINOR)** **util**: add tokens to parseArgs (John Gee) [#43459](nodejs/node#43459) - \[[`f32aec8a6d`](nodejs/node@f32aec8a6d)] - **util**: refactor to use validateObject (Kohei Ueno) [#43769](nodejs/node#43769) - \[[`d7cfd0c5ba`](nodejs/node@d7cfd0c5ba)] - **v8**: serialize BigInt64Array and BigUint64Array (Ben Noordhuis) [#43571](nodejs/node#43571)

PR-URL: #43971 Refs: #43949 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

PR-URL: nodejs/node#43971 Refs: nodejs/node#43949 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

mcollina approved these changes Jul 22, 2022

View reviewed changes

mcollina requested a review from ShogunPanda July 22, 2022 19:43

ShogunPanda approved these changes Jul 23, 2022

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2022

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 23, 2022

F3n67u mentioned this pull request Jul 23, 2022

Investigate why sequential/test-gc-http-client-onerror is flaky #43956

Closed

mcollina added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed needs-ci PRs that need a full CI run. labels Jul 23, 2022

mcollina added fast-track PRs that do not need to wait for 48 hours to land. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 23, 2022

lpinca mentioned this pull request Jul 23, 2022

test: mark sequential/test-gc-http-client-onerror as flaky #43957

Closed

lpinca reviewed Jul 23, 2022

View reviewed changes

Comment thread test/parallel/test-http-server-connection-list-when-close.js Outdated

lpinca reviewed Jul 23, 2022

View reviewed changes

Comment thread test/parallel/test-http-server-connection-list-when-close.js Outdated

Comment thread test/parallel/test-http-server-connection-list-when-close.js Outdated

F3n67u added fast-track PRs that do not need to wait for 48 hours to land. and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Jul 23, 2022

F3n67u added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 24, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 24, 2022

nodejs-github-bot merged commit da3949c into nodejs:main Jul 24, 2022

This was referenced Jul 24, 2022

test: remove test-gc-http-client-timeout from flaky list #43971

Merged

Server.closeIdleConnections break gc tests #43795

Closed

Investigate flaky test-gc-http-client-timeout #43638

Closed

This was referenced Jul 25, 2022

CI Reliability 2022-07-25 nodejs/reliability#331

Open

CI Reliability 2022-07-26 nodejs/reliability#332

Open

danielleadams pushed a commit that referenced this pull request Jul 26, 2022

http: fix http server connection list when close

25ec71d

PR-URL: #43949 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Feng Yu <F3n67u@outlook.com>

danielleadams mentioned this pull request Jul 26, 2022

v18.7.0 proposal #43993

Merged

github-actions Bot mentioned this pull request Jul 27, 2022

CI Reliability 2022-07-27 nodejs/reliability#333

Open

16 tasks

targos added the backport-requested-v16.x label Jul 31, 2022

targos added dont-land-on-v16.x and removed backport-requested-v16.x labels Jul 31, 2022

http: fix http server connection list when close by theanarkh · Pull Request #43949 · nodejs/node · GitHub

Uh oh!

Uh oh!

http: fix http server connection list when close - #43949

http: fix http server connection list when close#43949nodejs-github-bot merged 1 commit intonodejs:mainnodejs/node:mainfrom theanarkh:fix_server_connection_listtheanarkh/node:fix_server_connection_listCopy head branch name to clipboard

Conversation

theanarkh commented Jul 22, 2022 • edited by F3n67u Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 22, 2022

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

ShogunPanda left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jul 23, 2022

Uh oh!

mcollina commented Jul 23, 2022

Uh oh!

github-actions Bot commented Jul 23, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jul 23, 2022

Uh oh!

F3n67u commented Jul 23, 2022

Uh oh!

F3n67u commented Jul 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

node-stress-single-test comparison for sequential/test-gc-http-client-timeout

This pr (No Failure)

Main Branch (Failure)

Uh oh!

F3n67u commented Jul 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

node-stress-single-test comparison for test-gc-http-client & test-gc-http-client-onerror

This pr (No Failure)

Main Branch (Failure)

Uh oh!

theanarkh commented Jul 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

node-stress-single-test comparison for sequential/test-gc-http-client-timeout

This pr (No Failure)

Main Branch (Failure)

Uh oh!

nodejs-github-bot commented Jul 24, 2022

Uh oh!

nodejs-github-bot commented Jul 24, 2022

Uh oh!

targos commented Jul 31, 2022

Uh oh!

theanarkh commented Jul 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

http: fix http server connection list when close#43949
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
theanarkh:fix_server_connection_list

theanarkh commented Jul 22, 2022 •
edited by F3n67u
Loading

F3n67u commented Jul 24, 2022 •
edited
Loading

F3n67u commented Jul 24, 2022 •
edited
Loading

theanarkh commented Jul 24, 2022 •
edited
Loading