Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src,lib: the handle keeps loop alive in cluster rr mode #46161

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 26 additions & 7 deletions lib/internal/cluster/child.js
Expand Up @@ -16,6 +16,8 @@ const { owner_symbol } = require('internal/async_hooks').symbols;
const Worker = require('internal/cluster/worker');
const { internal, sendHelper } = require('internal/cluster/utils');
const { exitCodes: { kNoFailure } } = internalBinding('errors');
const { TIMEOUT_MAX } = require('internal/timers');
const { setInterval, clearInterval } = require('timers');

const cluster = new EventEmitter();
const handles = new SafeMap();
Expand Down Expand Up @@ -162,6 +164,21 @@ function rr(message, { indexesKey, index }, cb) {

let key = message.key;

let fakeHandle = null;

function ref() {
if (!fakeHandle) {
fakeHandle = setInterval(noop, TIMEOUT_MAX);
theanarkh marked this conversation as resolved.
Show resolved Hide resolved
}
}

function unref() {
if (fakeHandle) {
clearInterval(fakeHandle);
fakeHandle = null;
}
}

function listen(backlog) {
// TODO(bnoordhuis) Send a message to the primary that tells it to
// update the backlog size. The actual backlog should probably be
Expand All @@ -177,7 +194,11 @@ function rr(message, { indexesKey, index }, cb) {
// the primary.
if (key === undefined)
return;

unref();
// If the handle is the last handle in process,
// the parent process will delete the handle when worker process exits.
// So it is ok if the close message get lost.
// See the comments of https://github.com/nodejs/node/pull/46161
send({ act: 'close', key });
handles.delete(key);
removeIndexesKey(indexesKey, index);
Expand All @@ -191,12 +212,10 @@ function rr(message, { indexesKey, index }, cb) {
return 0;
}

// Faux handle. Mimics a TCPWrap with just enough fidelity to get away
// with it. Fools net.Server into thinking that it's backed by a real
// handle. Use a noop function for ref() and unref() because the control
// channel is going to keep the worker alive anyway.
const handle = { close, listen, ref: noop, unref: noop };

// Faux handle. net.Server is not associated with handle,
// so we control its state(ref or unref) by setInterval.
const handle = { close, listen, ref, unref };
handle.ref();
if (message.sockname) {
handle.getsockname = getsockname; // TCP handles only.
}
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-cluster-rr-handle-close.js
@@ -0,0 +1,18 @@
'use strict';

const common = require('../common');
const cluster = require('cluster');
const net = require('net');

cluster.schedulingPolicy = cluster.SCHED_RR;

if (cluster.isPrimary) {
const worker = cluster.fork();
worker.on('exit', common.mustCall());
} else {
const server = net.createServer(common.mustNotCall());
server.listen(0, common.mustCall(() => {
process.channel.unref();
server.close();
}));
}
23 changes: 23 additions & 0 deletions test/parallel/test-cluster-rr-handle-keep-loop-alive.js
@@ -0,0 +1,23 @@
'use strict';

const common = require('../common');
const cluster = require('cluster');
const net = require('net');
const assert = require('assert');

cluster.schedulingPolicy = cluster.SCHED_RR;

if (cluster.isPrimary) {
let exited = false;
const worker = cluster.fork();
worker.on('exit', () => {
exited = true;
});
setTimeout(() => {
assert.ok(!exited);
worker.kill();
}, 3000);
} else {
const server = net.createServer(common.mustNotCall());
server.listen(0, common.mustCall(() => process.channel.unref()));
}
20 changes: 20 additions & 0 deletions test/parallel/test-cluster-rr-handle-ref-unref.js
@@ -0,0 +1,20 @@
'use strict';

const common = require('../common');
const cluster = require('cluster');
const net = require('net');

cluster.schedulingPolicy = cluster.SCHED_RR;

if (cluster.isPrimary) {
const worker = cluster.fork();
worker.on('exit', common.mustCall());
} else {
const server = net.createServer(common.mustNotCall());
server.listen(0, common.mustCall(() => {
server.ref();
server.unref();
process.channel.unref();
}));
server.unref();
}