-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
cluster: remove handles when disconnecting worker
Due to the race window between the master's "disconnect" message and the worker's "handle received" message, connections sometimes got stuck in the pending handles queue when calling `worker.disconnect()` in the master process. The observable effect from the client's perspective was a TCP or HTTP connection that simply stalled. This commit fixes that by closing open handles in the master when the "disconnect" message is sent. Fixes: #3551 PR-URL: #3677 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Loading branch information
1 parent
fa4846e
commit 6c8dcc6
Showing
4 changed files
with
96 additions
and
23 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
'use strict'; | ||
|
||
// Used in tests. | ||
exports.handles = {}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
/* eslint-disable no-debugger */ | ||
// Flags: --expose_internals | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const cluster = require('cluster'); | ||
const net = require('net'); | ||
|
||
const Protocol = require('_debugger').Protocol; | ||
|
||
if (common.isWindows) { | ||
console.log('1..0 # Skipped: SCHED_RR not reliable on Windows'); | ||
return; | ||
} | ||
|
||
cluster.schedulingPolicy = cluster.SCHED_RR; | ||
|
||
// Worker sends back a "I'm here" message, then immediately suspends | ||
// inside the debugger. The master connects to the debug agent first, | ||
// connects to the TCP server second, then disconnects the worker and | ||
// unsuspends it again. The ultimate goal of this tortured exercise | ||
// is to make sure the connection is still sitting in the master's | ||
// pending handle queue. | ||
if (cluster.isMaster) { | ||
const handles = require('internal/cluster').handles; | ||
// FIXME(bnoordhuis) lib/cluster.js scans the execArgv arguments for | ||
// debugger flags and renumbers any port numbers it sees starting | ||
// from the default port 5858. Add a '.' that circumvents the | ||
// scanner but is ignored by atoi(3). Heinous hack. | ||
cluster.setupMaster({ execArgv: [`--debug=${common.PORT}.`] }); | ||
const worker = cluster.fork(); | ||
worker.on('message', common.mustCall(message => { | ||
assert.strictEqual(Array.isArray(message), true); | ||
assert.strictEqual(message[0], 'listening'); | ||
const address = message[1]; | ||
const host = address.address; | ||
const debugClient = net.connect({ host, port: common.PORT }); | ||
const protocol = new Protocol(); | ||
debugClient.setEncoding('utf8'); | ||
debugClient.on('data', data => protocol.execute(data)); | ||
debugClient.once('connect', common.mustCall(() => { | ||
protocol.onResponse = common.mustCall(res => { | ||
protocol.onResponse = () => {}; | ||
const conn = net.connect({ host, port: address.port }); | ||
conn.once('connect', common.mustCall(() => { | ||
conn.destroy(); | ||
assert.notDeepStrictEqual(handles, {}); | ||
worker.disconnect(); | ||
assert.deepStrictEqual(handles, {}); | ||
const req = protocol.serialize({ command: 'continue' }); | ||
debugClient.write(req); | ||
})); | ||
}); | ||
})); | ||
})); | ||
process.on('exit', () => assert.deepStrictEqual(handles, {})); | ||
} else { | ||
const server = net.createServer(socket => socket.pipe(socket)); | ||
server.listen(() => { | ||
process.send(['listening', server.address()]); | ||
debugger; | ||
}); | ||
process.on('disconnect', process.exit); | ||
} |