Skip to content

Commit

Permalink
test: Fix test-cluster-worker-exit.js for AIX
Browse files Browse the repository at this point in the history
test fails intermittently due to the assertion that the 'disconnect'
event should come before the 'exit' event. This is caused be the
non-deteministic behaviour of pollset_poll[1] on AIX
(see deps/uv/src/unix/aix.c). This API makes no garauntee for the order
in which file descriptors are returned. On linux epoll_wait[2] is used,
which also does not make a garauntee on order of file descriptors
returned. In the failing case we recieve our file descriptor with a
callback of uv__signal_event (which causes JavaScript to receive the
exit event) before our file descriptor with uv__stream_io as its
callback (which in turn causes JavaScript receive the disconnect event).
This change simply removes the assertion that the disconnect event
happens before exit event and processes the test regardless of which
event comes first.

[1] https://www-01.ibm.com/support/knowledgecenter/ssw_aix_71/com.ibm.ai
x.basetrf1/pollset.htm
[2] http://linux.die.net/man/2/epoll_pwait

PR-URL: #3666
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
Imran Iqbal authored and jasnell committed Nov 14, 2015
1 parent 3fea3cb commit 8b57b31
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions test/parallel/test-cluster-worker-exit.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,16 @@ if (cluster.isWorker) {
results.cluster_exitCode = worker.process.exitCode;
results.cluster_signalCode = worker.process.signalCode;
results.cluster_emitExit += 1;
assert.ok(results.cluster_emitDisconnect,
"cluster: 'exit' event before 'disconnect' event");
});

// Check worker events and properties
worker.on('disconnect', function() {
results.worker_emitDisconnect += 1;
results.worker_suicideMode = worker.suicide;
results.worker_state = worker.state;
if (results.worker_emitExit > 0) {
process.nextTick(function() { finish_test(); });
}
});

// Check that the worker died
Expand All @@ -77,10 +78,9 @@ if (cluster.isWorker) {
results.worker_signalCode = signalCode;
results.worker_emitExit += 1;
results.worker_died = !alive(worker.process.pid);
assert.ok(results.worker_emitDisconnect,
"worker: 'exit' event before 'disconnect' event");

process.nextTick(function() { finish_test(); });
if (results.worker_emitDisconnect > 0) {
process.nextTick(function() { finish_test(); });
}
});

var finish_test = function() {
Expand Down

2 comments on commit 8b57b31

@cjihrig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Fix' in the commit title should be all lowercase.

@jasnell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's it... early next week I'm writing myself a commit-lint tool for myself :-/

Please sign in to comment.