test: ensure finish is emitted before destroy#33137
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Adds a test to ensure that 'finish' is emitted before the socket is destroyed by allow half-open enforcer. Refs: nodejs@3c07b17#commitcomment-38810268
ronag commented Apr 28, 2020
review with ignore whitespace |
ronag commented Apr 28, 2020
This is a problem on v14. So we should backport this test together with a fix. Unfortunately the semver-major fix on master (#31806) cannot be backported. |
| socket.on('end',common.mustCall(()=>{ | ||
| assert(!socket.destroyed); | ||
| })); | ||
| socket.end('asd'); |
There was a problem hiding this comment.
calling end() here is the main difference with the previous test
| { | ||
| constserver=net.createServer(common.mustCall((socket)=>{ | ||
| socket.end(Buffer.alloc(1024)); |
There was a problem hiding this comment.
Is this needed now that the end() is also called on the client? Shouldn't this socket be resumed to consume the data sent by the client?
There was a problem hiding this comment.
Not sure, it does trigger the bug on v14 in the current form though...
nodejs-github-bot commented May 3, 2020
BridgeAR commented May 23, 2020
@nodejs/streams @nodejs/http @nodejs/http2 this needs some reviews. It's open for quite a while by now. |
nodejs-github-bot commented May 23, 2020
BridgeARforce-pushed the master branch 2 times, most recently from 8ae28ff to 2935f72Compare
jasnell commented Jun 19, 2020
Ping @nodejs/streams |
nodejs-github-bot commented Jun 21, 2020
Adds a test to ensure that 'finish' is emitted before the socket is destroyed by allow half-open enforcer. Refs: 3c07b17#commitcomment-38810268 PR-URL: #33137 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
ronag commented Jun 24, 2020
Landed in 2ccf15b |
7 participants
Adds a test to ensure that 'finish' is emitted
before the socket is destroyed by allow half-open
enforcer.
Refs: 3c07b17#commitcomment-38810268
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes