From 1358ddbcde51143ebb1caba3113afe1750025164 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 2 Jun 2018 12:34:06 +0200 Subject: [PATCH 1/3] test: refactor child-process-fork-net Split test-child-process-fork-net into test-child-process-fork-net-server and test-child-process-fork-net-socket. Rename test-child-process-fork-net2.js to test-child-process-fork-net.js. Refs: https://github.com/nodejs/node/pull/21012 --- .../test-child-process-fork-net-server.js | 145 ++++++++++ .../test-child-process-fork-net-socket.js | 92 +++++++ test/parallel/test-child-process-fork-net.js | 251 ++++++++---------- test/parallel/test-child-process-fork-net2.js | 164 ------------ 4 files changed, 342 insertions(+), 310 deletions(-) create mode 100644 test/parallel/test-child-process-fork-net-server.js create mode 100644 test/parallel/test-child-process-fork-net-socket.js delete mode 100644 test/parallel/test-child-process-fork-net2.js diff --git a/test/parallel/test-child-process-fork-net-server.js b/test/parallel/test-child-process-fork-net-server.js new file mode 100644 index 00000000000000..340b3075f9622a --- /dev/null +++ b/test/parallel/test-child-process-fork-net-server.js @@ -0,0 +1,145 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const fork = require('child_process').fork; +const net = require('net'); + +function ProgressTracker(missing, callback) { + this.missing = missing; + this.callback = callback; +} +ProgressTracker.prototype.done = function() { + this.missing -= 1; + this.check(); +}; +ProgressTracker.prototype.check = function() { + if (this.missing === 0) this.callback(); +}; + +if (process.argv[2] === 'child') { + + let serverScope; + + process.on('message', function onServer(msg, server) { + if (msg.what !== 'server') return; + process.removeListener('message', onServer); + + serverScope = server; + + server.on('connection', function(socket) { + console.log('CHILD: got connection'); + process.send({ what: 'connection' }); + socket.destroy(); + }); + + // Start making connection from parent. + console.log('CHILD: server listening'); + process.send({ what: 'listening' }); + }); + + process.on('message', function onClose(msg) { + if (msg.what !== 'close') return; + process.removeListener('message', onClose); + + serverScope.on('close', function() { + process.send({ what: 'close' }); + }); + serverScope.close(); + }); + + process.send({ what: 'ready' }); +} else { + + const child = fork(process.argv[1], ['child']); + + child.on('exit', common.mustCall(function(code, signal) { + const message = `CHILD: died with ${code}, ${signal}`; + assert.strictEqual(code, 0, message); + })); + + // Send net.Server to child and test by connecting. + function testServer(callback) { + + // Destroy server execute callback when done. + const progress = new ProgressTracker(2, function() { + server.on('close', function() { + console.log('PARENT: server closed'); + child.send({ what: 'close' }); + }); + server.close(); + }); + + // We expect 4 connections and close events. + const connections = new ProgressTracker(4, progress.done.bind(progress)); + const closed = new ProgressTracker(4, progress.done.bind(progress)); + + // Create server and send it to child. + const server = net.createServer(); + server.on('connection', function(socket) { + console.log('PARENT: got connection'); + socket.destroy(); + connections.done(); + }); + server.on('listening', function() { + console.log('PARENT: server listening'); + child.send({ what: 'server' }, server); + }); + server.listen(0); + + // Handle client messages. + function messageHandlers(msg) { + + if (msg.what === 'listening') { + // Make connections. + let socket; + for (let i = 0; i < 4; i++) { + socket = net.connect(server.address().port, function() { + console.log('CLIENT: connected'); + }); + socket.on('close', function() { + closed.done(); + console.log('CLIENT: closed'); + }); + } + + } else if (msg.what === 'connection') { + // child got connection + connections.done(); + } else if (msg.what === 'close') { + child.removeListener('message', messageHandlers); + callback(); + } + } + + child.on('message', messageHandlers); + } + + // Create server and send it to child. + child.on('message', function onReady(msg) { + if (msg.what !== 'ready') return; + child.removeListener('message', onReady); + + testServer(common.mustCall()); + }); +} diff --git a/test/parallel/test-child-process-fork-net-socket.js b/test/parallel/test-child-process-fork-net-socket.js new file mode 100644 index 00000000000000..331c3424008366 --- /dev/null +++ b/test/parallel/test-child-process-fork-net-socket.js @@ -0,0 +1,92 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const fork = require('child_process').fork; +const net = require('net'); + +if (process.argv[2] === 'child') { + + process.on('message', function onSocket(msg, socket) { + if (msg.what !== 'socket') return; + process.removeListener('message', onSocket); + socket.end('echo'); + console.log('CHILD: got socket'); + }); + + process.send({ what: 'ready' }); +} else { + + const child = fork(process.argv[1], ['child']); + + child.on('exit', common.mustCall(function(code, signal) { + const message = `CHILD: died with ${code}, ${signal}`; + assert.strictEqual(code, 0, message); + })); + + // Send net.Socket to child. + function testSocket(callback) { + + // Create a new server and connect to it, + // but the socket will be handled by the child. + const server = net.createServer(); + server.on('connection', function(socket) { + socket.on('close', function() { + console.log('CLIENT: socket closed'); + }); + child.send({ what: 'socket' }, socket); + }); + server.on('close', function() { + console.log('PARENT: server closed'); + callback(); + }); + // Don't listen on the same port, because SmartOS sometimes says + // that the server's fd is closed, but it still cannot listen + // on the same port again. + // + // An isolated test for this would be lovely, but for now, this + // will have to do. + server.listen(0, function() { + console.log('testSocket, listening'); + const connect = net.connect(server.address().port); + let store = ''; + connect.on('data', function(chunk) { + store += chunk; + console.log('CLIENT: got data'); + }); + connect.on('close', function() { + console.log('CLIENT: closed'); + assert.strictEqual(store, 'echo'); + server.close(); + }); + }); + } + + // Create socket and send it to child. + child.on('message', function onReady(msg) { + if (msg.what !== 'ready') return; + child.removeListener('message', onReady); + + testSocket(common.mustCall()); + }); +} diff --git a/test/parallel/test-child-process-fork-net.js b/test/parallel/test-child-process-fork-net.js index cfc8f564d2a03a..babde351d3f05a 100644 --- a/test/parallel/test-child-process-fork-net.js +++ b/test/parallel/test-child-process-fork-net.js @@ -24,182 +24,141 @@ const common = require('../common'); const assert = require('assert'); const fork = require('child_process').fork; const net = require('net'); - -function ProgressTracker(missing, callback) { - this.missing = missing; - this.callback = callback; -} -ProgressTracker.prototype.done = function() { - this.missing -= 1; - this.check(); -}; -ProgressTracker.prototype.check = function() { - if (this.missing === 0) this.callback(); -}; +const count = 12; if (process.argv[2] === 'child') { + const needEnd = []; + const id = process.argv[3]; + + process.on('message', function(m, socket) { + if (!socket) return; - let serverScope; + console.error(`[${id}] got socket ${m}`); - process.on('message', function onServer(msg, server) { - if (msg.what !== 'server') return; - process.removeListener('message', onServer); + // will call .end('end') or .write('write'); + socket[m](m); - serverScope = server; + socket.resume(); - server.on('connection', function(socket) { - console.log('CHILD: got connection'); - process.send({ what: 'connection' }); - socket.destroy(); + socket.on('data', function() { + console.error(`[${id}] socket.data ${m}`); }); - // Start making connection from parent. - console.log('CHILD: server listening'); - process.send({ what: 'listening' }); - }); + socket.on('end', function() { + console.error(`[${id}] socket.end ${m}`); + }); - process.on('message', function onClose(msg) { - if (msg.what !== 'close') return; - process.removeListener('message', onClose); + // store the unfinished socket + if (m === 'write') { + needEnd.push(socket); + } - serverScope.on('close', function() { - process.send({ what: 'close' }); + socket.on('close', function(had_error) { + console.error(`[${id}] socket.close ${had_error} ${m}`); }); - serverScope.close(); - }); - process.on('message', function onSocket(msg, socket) { - if (msg.what !== 'socket') return; - process.removeListener('message', onSocket); - socket.end('echo'); - console.log('CHILD: got socket'); + socket.on('finish', function() { + console.error(`[${id}] socket finished ${m}`); + }); }); - process.send({ what: 'ready' }); -} else { + process.on('message', function(m) { + if (m !== 'close') return; + console.error(`[${id}] got close message`); + needEnd.forEach(function(endMe, i) { + console.error(`[${id}] ending ${i}/${needEnd.length}`); + endMe.end('end'); + }); + }); - const child = fork(process.argv[1], ['child']); + process.on('disconnect', function() { + console.error(`[${id}] process disconnect, ending`); + needEnd.forEach(function(endMe, i) { + console.error(`[${id}] ending ${i}/${needEnd.length}`); + endMe.end('end'); + }); + }); - child.on('exit', common.mustCall(function(code, signal) { - const message = `CHILD: died with ${code}, ${signal}`; - assert.strictEqual(code, 0, message); - })); +} else { - // Send net.Server to child and test by connecting. - function testServer(callback) { + const child1 = fork(process.argv[1], ['child', '1']); + const child2 = fork(process.argv[1], ['child', '2']); + const child3 = fork(process.argv[1], ['child', '3']); + + const server = net.createServer(); + + let connected = 0; + let closed = 0; + server.on('connection', function(socket) { + switch (connected % 6) { + case 0: + child1.send('end', socket); break; + case 1: + child1.send('write', socket); break; + case 2: + child2.send('end', socket); break; + case 3: + child2.send('write', socket); break; + case 4: + child3.send('end', socket); break; + case 5: + child3.send('write', socket); break; + } + connected += 1; - // Destroy server execute callback when done. - const progress = new ProgressTracker(2, function() { - server.on('close', function() { - console.log('PARENT: server closed'); - child.send({ what: 'close' }); - }); - server.close(); + socket.once('close', function() { + console.log(`[m] socket closed, total ${++closed}`); }); - // We expect 4 connections and close events. - const connections = new ProgressTracker(4, progress.done.bind(progress)); - const closed = new ProgressTracker(4, progress.done.bind(progress)); - - // Create server and send it to child. - const server = net.createServer(); - server.on('connection', function(socket) { - console.log('PARENT: got connection'); - socket.destroy(); - connections.done(); - }); - server.on('listening', function() { - console.log('PARENT: server listening'); - child.send({ what: 'server' }, server); - }); - server.listen(0); - - // Handle client messages. - function messageHandlers(msg) { - - if (msg.what === 'listening') { - // Make connections. - let socket; - for (let i = 0; i < 4; i++) { - socket = net.connect(server.address().port, function() { - console.log('CLIENT: connected'); - }); - socket.on('close', function() { - closed.done(); - console.log('CLIENT: closed'); - }); - } - - } else if (msg.what === 'connection') { - // child got connection - connections.done(); - } else if (msg.what === 'close') { - child.removeListener('message', messageHandlers); - callback(); - } + if (connected === count) { + closeServer(); } + }); - child.on('message', messageHandlers); - } - - // Send net.Socket to child. - function testSocket(callback) { + let disconnected = 0; + server.on('listening', function() { - // Create a new server and connect to it, - // but the socket will be handled by the child. - const server = net.createServer(); - server.on('connection', function(socket) { - socket.on('close', function() { - console.log('CLIENT: socket closed'); + let j = count; + while (j--) { + const client = net.connect(this.address().port, '127.0.0.1'); + client.on('error', function() { + // This can happen if we kill the child too early. + // The client should still get a close event afterwards. + console.error('[m] CLIENT: error event'); }); - child.send({ what: 'socket' }, socket); - }); - server.on('close', function() { - console.log('PARENT: server closed'); - callback(); - }); - // Don't listen on the same port, because SmartOS sometimes says - // that the server's fd is closed, but it still cannot listen - // on the same port again. - // - // An isolated test for this would be lovely, but for now, this - // will have to do. - server.listen(0, function() { - console.log('testSocket, listening'); - const connect = net.connect(server.address().port); - let store = ''; - connect.on('data', function(chunk) { - store += chunk; - console.log('CLIENT: got data'); - }); - connect.on('close', function() { - console.log('CLIENT: closed'); - assert.strictEqual(store, 'echo'); - server.close(); + client.on('close', function() { + console.error('[m] CLIENT: close event'); + disconnected += 1; }); - }); - } + client.resume(); + } + }); - // Create server and send it to child. - let serverSuccess = false; - let socketSuccess = false; - child.on('message', function onReady(msg) { - if (msg.what !== 'ready') return; - child.removeListener('message', onReady); + let closeEmitted = false; + server.on('close', common.mustCall(function() { + closeEmitted = true; - testServer(function() { - serverSuccess = true; + child1.kill(); + child2.kill(); + child3.kill(); + })); - testSocket(function() { - socketSuccess = true; - }); - }); + server.listen(0, '127.0.0.1'); - }); + function closeServer() { + server.close(); + + setTimeout(function() { + assert(!closeEmitted); + child1.send('close'); + child2.send('close'); + child3.disconnect(); + }, 200); + } process.on('exit', function() { - assert.ok(serverSuccess); - assert.ok(socketSuccess); + assert.strictEqual(server._workers.length, 0); + assert.strictEqual(disconnected, count); + assert.strictEqual(connected, count); }); - } diff --git a/test/parallel/test-child-process-fork-net2.js b/test/parallel/test-child-process-fork-net2.js deleted file mode 100644 index babde351d3f05a..00000000000000 --- a/test/parallel/test-child-process-fork-net2.js +++ /dev/null @@ -1,164 +0,0 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const fork = require('child_process').fork; -const net = require('net'); -const count = 12; - -if (process.argv[2] === 'child') { - const needEnd = []; - const id = process.argv[3]; - - process.on('message', function(m, socket) { - if (!socket) return; - - console.error(`[${id}] got socket ${m}`); - - // will call .end('end') or .write('write'); - socket[m](m); - - socket.resume(); - - socket.on('data', function() { - console.error(`[${id}] socket.data ${m}`); - }); - - socket.on('end', function() { - console.error(`[${id}] socket.end ${m}`); - }); - - // store the unfinished socket - if (m === 'write') { - needEnd.push(socket); - } - - socket.on('close', function(had_error) { - console.error(`[${id}] socket.close ${had_error} ${m}`); - }); - - socket.on('finish', function() { - console.error(`[${id}] socket finished ${m}`); - }); - }); - - process.on('message', function(m) { - if (m !== 'close') return; - console.error(`[${id}] got close message`); - needEnd.forEach(function(endMe, i) { - console.error(`[${id}] ending ${i}/${needEnd.length}`); - endMe.end('end'); - }); - }); - - process.on('disconnect', function() { - console.error(`[${id}] process disconnect, ending`); - needEnd.forEach(function(endMe, i) { - console.error(`[${id}] ending ${i}/${needEnd.length}`); - endMe.end('end'); - }); - }); - -} else { - - const child1 = fork(process.argv[1], ['child', '1']); - const child2 = fork(process.argv[1], ['child', '2']); - const child3 = fork(process.argv[1], ['child', '3']); - - const server = net.createServer(); - - let connected = 0; - let closed = 0; - server.on('connection', function(socket) { - switch (connected % 6) { - case 0: - child1.send('end', socket); break; - case 1: - child1.send('write', socket); break; - case 2: - child2.send('end', socket); break; - case 3: - child2.send('write', socket); break; - case 4: - child3.send('end', socket); break; - case 5: - child3.send('write', socket); break; - } - connected += 1; - - socket.once('close', function() { - console.log(`[m] socket closed, total ${++closed}`); - }); - - if (connected === count) { - closeServer(); - } - }); - - let disconnected = 0; - server.on('listening', function() { - - let j = count; - while (j--) { - const client = net.connect(this.address().port, '127.0.0.1'); - client.on('error', function() { - // This can happen if we kill the child too early. - // The client should still get a close event afterwards. - console.error('[m] CLIENT: error event'); - }); - client.on('close', function() { - console.error('[m] CLIENT: close event'); - disconnected += 1; - }); - client.resume(); - } - }); - - let closeEmitted = false; - server.on('close', common.mustCall(function() { - closeEmitted = true; - - child1.kill(); - child2.kill(); - child3.kill(); - })); - - server.listen(0, '127.0.0.1'); - - function closeServer() { - server.close(); - - setTimeout(function() { - assert(!closeEmitted); - child1.send('close'); - child2.send('close'); - child3.disconnect(); - }, 200); - } - - process.on('exit', function() { - assert.strictEqual(server._workers.length, 0); - assert.strictEqual(disconnected, count); - assert.strictEqual(connected, count); - }); -} From da719115bbe9aba4ae177a1f4fdd576b26756fb5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 2 Jun 2018 13:07:29 +0200 Subject: [PATCH 2/3] squash! --- test/parallel/parallel.status | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index e4b757bbd3dac9..c790aeaf5c0c15 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -9,7 +9,7 @@ prefix parallel test-postmortem-metadata: PASS,FLAKY [$system==win32] -test-child-process-fork-net: PASS,FLAKY +test-child-process-fork-net-socket: PASS,FLAKY [$system==linux] From 3c7a544dd8a756b1a32ade56ba85ab552a765c13 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 3 Jun 2018 06:15:12 +0200 Subject: [PATCH 3/3] squash! remove obsolete comment --- test/parallel/test-child-process-fork-net-socket.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/parallel/test-child-process-fork-net-socket.js b/test/parallel/test-child-process-fork-net-socket.js index 331c3424008366..8e3d9ee16e2e56 100644 --- a/test/parallel/test-child-process-fork-net-socket.js +++ b/test/parallel/test-child-process-fork-net-socket.js @@ -60,12 +60,7 @@ if (process.argv[2] === 'child') { console.log('PARENT: server closed'); callback(); }); - // Don't listen on the same port, because SmartOS sometimes says - // that the server's fd is closed, but it still cannot listen - // on the same port again. - // - // An isolated test for this would be lovely, but for now, this - // will have to do. + server.listen(0, function() { console.log('testSocket, listening'); const connect = net.connect(server.address().port);