From 3bf1c0801462d41585c1295d9fb5d8d579f2f1a9 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:03:42 +0100 Subject: [PATCH 1/6] net: wait for shutdown to complete before closing When not allowing half open, handle.close would be invoked before shutdown has been called and completed causing a potential data race. Fixes: https://github.com/nodejs/node/issues/32486#issuecomment-604072559 --- lib/net.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/net.js b/lib/net.js index 4f597a1008a88d..ee0ffebe0ec45e 100644 --- a/lib/net.js +++ b/lib/net.js @@ -630,9 +630,9 @@ function onReadableStreamEnd() { this.write = writeAfterFIN; if (this.writable) this.end(); - } - - if (!this.destroyed && !this.writable && !this.writableLength) + else if (!this.writableLength) + this.destroy(); + } else if (!this.destroyed && !this.writable && !this.writableLength) this.destroy(); } From 93c6f755653733298c3dabee8eceae4ba953cd68 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:26:55 +0100 Subject: [PATCH 2/6] fixup: test --- test/parallel/test-net-allow-half-open.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/parallel/test-net-allow-half-open.js diff --git a/test/parallel/test-net-allow-half-open.js b/test/parallel/test-net-allow-half-open.js new file mode 100644 index 00000000000000..97c1ed820ec8f9 --- /dev/null +++ b/test/parallel/test-net-allow-half-open.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const server = net.createServer(common.mustCall((socket) => { + socket.end(Buffer.alloc(1024)); +})).listen(0, common.mustCall(() => { + const socket = net.connect(server.address().port); + assert(socket.allowHalfOpen === false); + socket.resume(); + socket.on('end', common.mustCall(() => { + process.nextTick(() => { + assert(!socket.destroyed); + server.close(); + }) + })); + socket.on('close', common.mustCall()); +})); From c485b6a563f3f6ef493205b51e1865f4aca82305 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:27:36 +0100 Subject: [PATCH 3/6] fixup: test --- test/async-hooks/test-graph.tls-write.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/async-hooks/test-graph.tls-write.js b/test/async-hooks/test-graph.tls-write.js index f8bee6a879d0b4..078dd27c36088c 100644 --- a/test/async-hooks/test-graph.tls-write.js +++ b/test/async-hooks/test-graph.tls-write.js @@ -65,9 +65,7 @@ function onexit() { { type: 'TCPCONNECTWRAP', id: 'tcpconnect:1', triggerAsyncId: 'tcp:1' }, { type: 'TCPWRAP', id: 'tcp:2', triggerAsyncId: 'tcpserver:1' }, - { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcpserver:1' }, - { type: 'Immediate', id: 'immediate:1', triggerAsyncId: 'tcp:2' }, - { type: 'Immediate', id: 'immediate:2', triggerAsyncId: 'tcp:1' }, + { type: 'TLSWRAP', id: 'tls:2', triggerAsyncId: 'tcpserver:1' } ] ); } From 871866ea805b3ba29a2a00fc6d9c99b4318b80e0 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:30:17 +0100 Subject: [PATCH 4/6] fixup: tst --- test/parallel/test-net-allow-half-open.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/parallel/test-net-allow-half-open.js b/test/parallel/test-net-allow-half-open.js index 97c1ed820ec8f9..0a530257e3f977 100644 --- a/test/parallel/test-net-allow-half-open.js +++ b/test/parallel/test-net-allow-half-open.js @@ -12,9 +12,14 @@ const server = net.createServer(common.mustCall((socket) => { socket.resume(); socket.on('end', common.mustCall(() => { process.nextTick(() => { + // Ensure socket is not destroyed straight away + // without proper shutdown. assert(!socket.destroyed); server.close(); }) })); + socket.on('finish', common.mustCall(() => { + assert(!socket.destroyed); + })); socket.on('close', common.mustCall()); })); From fb389dc71214ae3cf19cb2819f471f1242655a93 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:33:43 +0100 Subject: [PATCH 5/6] fixup: test --- test/parallel/test-net-allow-half-open.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-net-allow-half-open.js b/test/parallel/test-net-allow-half-open.js index 0a530257e3f977..8f68c0105a15dc 100644 --- a/test/parallel/test-net-allow-half-open.js +++ b/test/parallel/test-net-allow-half-open.js @@ -8,7 +8,7 @@ const server = net.createServer(common.mustCall((socket) => { socket.end(Buffer.alloc(1024)); })).listen(0, common.mustCall(() => { const socket = net.connect(server.address().port); - assert(socket.allowHalfOpen === false); + assert.strictEqual(socket.allowHalfOpen, false); socket.resume(); socket.on('end', common.mustCall(() => { process.nextTick(() => { @@ -16,7 +16,7 @@ const server = net.createServer(common.mustCall((socket) => { // without proper shutdown. assert(!socket.destroyed); server.close(); - }) + }); })); socket.on('finish', common.mustCall(() => { assert(!socket.destroyed); From a9382cbe13d8aff2058b5167c6edd3ef54d5f044 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 25 Mar 2020 22:51:14 +0100 Subject: [PATCH 6/6] fixup: apply win10 fix --- lib/internal/stream_base_commons.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/internal/stream_base_commons.js b/lib/internal/stream_base_commons.js index 800ba4cefd6370..2a31221f11d2d6 100644 --- a/lib/internal/stream_base_commons.js +++ b/lib/internal/stream_base_commons.js @@ -213,6 +213,16 @@ function onStreamRead(arrayBuffer) { if (stream[kMaybeDestroy]) stream.on('end', stream[kMaybeDestroy]); + // TODO(ronag): Without this `readStop`, `onStreamRead` + // will be called once more (i.e. after Readable.ended) + // on Windows causing a ECONNRESET, failing the + // test-https-truncate test. + if (handle.readStop) { + const err = handle.readStop(); + if (err) + return stream.destroy(errnoException(err, 'read')); + } + // Push a null to signal the end of data. // Do it before `maybeDestroy` for correct order of events: // `end` -> `close`