-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
worker: add name for worker #59213
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
worker: add name for worker #59213
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -73,6 +73,7 @@ const { | |||||||||
| isInternalThread, | ||||||||||
| resourceLimits: resourceLimitsRaw, | ||||||||||
| threadId, | ||||||||||
| threadName, | ||||||||||
| Worker: WorkerImpl, | ||||||||||
| kMaxYoungGenerationSizeMb, | ||||||||||
| kMaxOldGenerationSizeMb, | ||||||||||
|
|
@@ -425,6 +426,12 @@ class Worker extends EventEmitter { | |||||||||
| return this[kHandle].threadId; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| get threadName() { | ||||||||||
| if (this[kHandle] === null) return null; | ||||||||||
|
|
||||||||||
| return this[kHandle].threadName; | ||||||||||
|
Comment on lines
+430
to
+432
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe you could simplify this a bit as...
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will optimize it in another pr.Thanks. |
||||||||||
| } | ||||||||||
|
|
||||||||||
| get stdin() { | ||||||||||
| return this[kParentSideStdio].stdin; | ||||||||||
| } | ||||||||||
|
|
@@ -589,6 +596,7 @@ module.exports = { | |||||||||
| getEnvironmentData, | ||||||||||
| assignEnvironmentData, | ||||||||||
| threadId, | ||||||||||
| threadName, | ||||||||||
| InternalWorker, | ||||||||||
| Worker, | ||||||||||
| }; | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
| const assert = require('assert'); | ||
| const { Worker, threadName, workerData } = require('worker_threads'); | ||
|
|
||
| const name = 'test-worker-thread-name'; | ||
|
|
||
| if (workerData?.isWorker) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the const { Worker, isMainThread } = require('worker_threads');
if (!isMainThread) {
// This is running in a worker
} else {
// This is running in the main thread
}
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. workerData?.isWorker is designed to support the creation of worker within worker.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that actually necessary tho? I would just make this test as a whole not run in a worker.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also need to support this use case, so should we support this kind of test ? |
||
| assert.strictEqual(threadName, name); | ||
| process.exit(0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be no reason for |
||
| } else { | ||
| const w = new Worker(__filename, { name, workerData: { isWorker: true } }); | ||
| assert.strictEqual(w.threadName, name); | ||
| w.on('exit', common.mustCall(() => { | ||
| assert.strictEqual(w.threadName, null); | ||
| })); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit since I see we don't do this with the other properties here so feel free to ignore, but it would be ideal if the docs were clear that this is a read-only property. (same goes for the other read-only properties here)