Skip to content

Commit

Permalink
worker: fix nullptr deref after MessagePort deser failure
Browse files Browse the repository at this point in the history
This would previously always have crashed when deserializing
a `MessagePort` fails, because there was always at least one
`nullptr` entry in the vector.

PR-URL: #25076
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
addaleax authored and BethGriggs committed Jul 16, 2019
1 parent 0dee607 commit 14090b5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/node_messaging.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ MaybeLocal<Value> Message::Deserialize(Environment* env,
if (ports[i] == nullptr) {
for (MessagePort* port : ports) {
// This will eventually release the MessagePort object itself.
port->Close();
if (port != nullptr)
port->Close();
}
return MaybeLocal<Value>();
}
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-worker-messageport-transfer-terminate.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Flags: --experimental-worker
'use strict';
require('../common');
const { Worker, MessageChannel } = require('worker_threads');

// Check the interaction of calling .terminate() while transferring
// MessagePort objects; in particular, that it does not crash the process.

for (let i = 0; i < 10; ++i) {
const w = new Worker(
"require('worker_threads').parentPort.on('message', () => {})",
{ eval: true });
setImmediate(() => {
const port = new MessageChannel().port1;
w.postMessage({ port }, [ port ]);
w.terminate();
});
}

0 comments on commit 14090b5

Please sign in to comment.