worker: no throw on property access/postMessage after termination

chjj · targos · commit 5d2e0649739d · 2019-02-10T11:59:30.000+01:00
PR-URL: #25871 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@hiroppy.me> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/lib/internal/worker.js b/lib/internal/worker.js
@@ -184,6 +184,8 @@ class Worker extends EventEmitter {
   }
 
   postMessage(...args) {
+    if (this[kPublicPort] === null) return;
+
     this[kPublicPort].postMessage(...args);
   }
 
@@ -219,14 +221,20 @@ class Worker extends EventEmitter {
   }
 
   get stdin() {
+    if (this[kParentSideStdio] === null) return null;
+
     return this[kParentSideStdio].stdin;
   }
 
   get stdout() {
+    if (this[kParentSideStdio] === null) return null;
+
     return this[kParentSideStdio].stdout;
   }
 
   get stderr() {
+    if (this[kParentSideStdio] === null) return null;
+
     return this[kParentSideStdio].stderr;
   }
 }
diff --git a/test/parallel/test-worker-safe-getters.js b/test/parallel/test-worker-safe-getters.js
@@ -0,0 +1,38 @@
+'use strict';
+
+const common = require('../common');
+
+const assert = require('assert');
+const { Worker, isMainThread } = require('worker_threads');
+
+if (isMainThread) {
+  const w = new Worker(__filename, {
+    stdin: true,
+    stdout: true,
+    stderr: true
+  });
+
+  w.on('exit', common.mustCall((code) => {
+    assert.strictEqual(code, 0);
+
+    // `postMessage` should not throw after termination
+    // (this mimics the browser behavior).
+    w.postMessage('foobar');
+    w.ref();
+    w.unref();
+
+    // Although not browser specific, probably wise to
+    // make sure the stream getters don't throw either.
+    w.stdin;
+    w.stdout;
+    w.stderr;
+
+    // Sanity check.
+    assert.strictEqual(w.threadId, -1);
+    assert.strictEqual(w.stdin, null);
+    assert.strictEqual(w.stdout, null);
+    assert.strictEqual(w.stderr, null);
+  }));
+} else {
+  process.exit(0);
+}

Original file line number	Diff line number	Diff line change
`@@ -184,6 +184,8 @@ class Worker extends EventEmitter {`
`184`	`184`	`}`
`185`	`185`
`186`	`186`	`postMessage(...args) {`
	`187`	`+ if (this[kPublicPort] === null) return;`
	`188`	`+`
`187`	`189`	`this[kPublicPort].postMessage(...args);`
`188`	`190`	`}`
`189`	`191`
`@@ -219,14 +221,20 @@ class Worker extends EventEmitter {`
`219`	`221`	`}`
`220`	`222`
`221`	`223`	`get stdin() {`
	`224`	`+ if (this[kParentSideStdio] === null) return null;`
	`225`	`+`
`222`	`226`	`return this[kParentSideStdio].stdin;`
`223`	`227`	`}`
`224`	`228`
`225`	`229`	`get stdout() {`
	`230`	`+ if (this[kParentSideStdio] === null) return null;`
	`231`	`+`
`226`	`232`	`return this[kParentSideStdio].stdout;`
`227`	`233`	`}`
`228`	`234`
`229`	`235`	`get stderr() {`
	`236`	`+ if (this[kParentSideStdio] === null) return null;`
	`237`	`+`
`230`	`238`	`return this[kParentSideStdio].stderr;`
`231`	`239`	`}`
`232`	`240`	`}`