Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions doc/api/worker_threads.md
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,17 @@ An integer identifier for the current thread. On the corresponding worker object
(if there is any), it is available as [`worker.threadId`][].
This value is unique for each [`Worker`][] instance inside a single process.

## `worker.threadName`
Copy link
Member

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)


<!-- YAML
added: REPLACEME
-->

* {string|null}

A string identifier for the current thread or null if the thread is not running.
On the corresponding worker object (if there is any), it is available as [`worker.threadName`][].

## `worker.workerData`

<!-- YAML
Expand Down Expand Up @@ -2015,6 +2026,17 @@ An integer identifier for the referenced thread. Inside the worker thread,
it is available as [`require('node:worker_threads').threadId`][].
This value is unique for each `Worker` instance inside a single process.

### `worker.threadName`

<!-- YAML
added: REPLACEME
-->

* {string|null}

A string identifier for the referenced thread or null if the thread is not running.
Inside the worker thread, it is available as [`require('node:worker_threads').threadName`][].

### `worker.unref()`

<!-- YAML
Expand Down Expand Up @@ -2143,6 +2165,7 @@ thread spawned will spawn another until the application crashes.
[`require('node:worker_threads').parentPort.postMessage()`]: #workerpostmessagevalue-transferlist
[`require('node:worker_threads').parentPort`]: #workerparentport
[`require('node:worker_threads').threadId`]: #workerthreadid
[`require('node:worker_threads').threadName`]: #workerthreadname
[`require('node:worker_threads').workerData`]: #workerworkerdata
[`trace_events`]: tracing.md
[`v8.getHeapSnapshot()`]: v8.md#v8getheapsnapshotoptions
Expand All @@ -2153,6 +2176,7 @@ thread spawned will spawn another until the application crashes.
[`worker.postMessage()`]: #workerpostmessagevalue-transferlist
[`worker.terminate()`]: #workerterminate
[`worker.threadId`]: #workerthreadid_1
[`worker.threadName`]: #workerthreadname_1
[async-resource-worker-pool]: async_context.md#using-asyncresource-for-a-worker-thread-pool
[browser `LockManager`]: https://developer.mozilla.org/en-US/docs/Web/API/LockManager
[browser `MessagePort`]: https://developer.mozilla.org/en-US/docs/Web/API/MessagePort
Expand Down
8 changes: 8 additions & 0 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ const {
isInternalThread,
resourceLimits: resourceLimitsRaw,
threadId,
threadName,
Worker: WorkerImpl,
kMaxYoungGenerationSizeMb,
kMaxOldGenerationSizeMb,
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

I believe you could simplify this a bit as...

Suggested change
if (this[kHandle] === null) return null;
return this[kHandle].threadName;
return this[kHandle]?.threadName || null;

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
}
Expand Down Expand Up @@ -589,6 +596,7 @@ module.exports = {
getEnvironmentData,
assignEnvironmentData,
threadId,
threadName,
InternalWorker,
Worker,
};
2 changes: 2 additions & 0 deletions lib/worker_threads.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const {
setEnvironmentData,
getEnvironmentData,
threadId,
threadName,
Worker,
} = require('internal/worker');

Expand Down Expand Up @@ -44,6 +45,7 @@ module.exports = {
resourceLimits,
postMessageToThread,
threadId,
threadName,
SHARE_ENV,
Worker,
parentPort: null,
Expand Down
22 changes: 21 additions & 1 deletion src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,25 @@ Environment* CreateEnvironment(
EnvironmentFlags::Flags flags,
ThreadId thread_id,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle) {
return CreateEnvironment(isolate_data,
context,
args,
exec_args,
flags,
thread_id,
std::move(inspector_parent_handle),
{});
}

Environment* CreateEnvironment(
IsolateData* isolate_data,
Local<Context> context,
const std::vector<std::string>& args,
const std::vector<std::string>& exec_args,
EnvironmentFlags::Flags flags,
ThreadId thread_id,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle,
std::string_view thread_name) {
Isolate* isolate = isolate_data->isolate();

Isolate::Scope isolate_scope(isolate);
Expand All @@ -434,7 +453,8 @@ Environment* CreateEnvironment(
exec_args,
env_snapshot_info,
flags,
thread_id);
thread_id,
thread_name);
CHECK_NOT_NULL(env);

if (use_snapshot) {
Expand Down
4 changes: 4 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,10 @@ inline uint64_t Environment::thread_id() const {
return thread_id_;
}

inline std::string_view Environment::thread_name() const {
return thread_name_;
}

inline worker::Worker* Environment::worker_context() const {
return isolate_data()->worker_context();
}
Expand Down
6 changes: 4 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,8 @@ Environment::Environment(IsolateData* isolate_data,
const std::vector<std::string>& exec_args,
const EnvSerializeInfo* env_info,
EnvironmentFlags::Flags flags,
ThreadId thread_id)
ThreadId thread_id,
std::string_view thread_name)
: isolate_(isolate),
external_memory_accounter_(new ExternalMemoryAccounter()),
isolate_data_(isolate_data),
Expand All @@ -811,7 +812,8 @@ Environment::Environment(IsolateData* isolate_data,
flags_(flags),
thread_id_(thread_id.id == static_cast<uint64_t>(-1)
? AllocateEnvironmentThreadId().id
: thread_id.id) {
: thread_id.id),
thread_name_(thread_name) {
if (!is_main_thread()) {
// If this is a Worker thread, we can always safely use the parent's
// Isolate's code cache because of the shared read-only heap.
Expand Down
5 changes: 4 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,8 @@ class Environment final : public MemoryRetainer {
const std::vector<std::string>& exec_args,
const EnvSerializeInfo* env_info,
EnvironmentFlags::Flags flags,
ThreadId thread_id);
ThreadId thread_id,
std::string_view thread_name = "");
void InitializeMainContext(v8::Local<v8::Context> context,
const EnvSerializeInfo* env_info);
~Environment() override;
Expand Down Expand Up @@ -807,6 +808,7 @@ class Environment final : public MemoryRetainer {
inline bool should_start_debug_signal_handler() const;
inline bool no_browser_globals() const;
inline uint64_t thread_id() const;
inline std::string_view thread_name() const;
inline worker::Worker* worker_context() const;
Environment* worker_parent_env() const;
inline void add_sub_worker_context(worker::Worker* context);
Expand Down Expand Up @@ -1172,6 +1174,7 @@ class Environment final : public MemoryRetainer {

uint64_t flags_;
uint64_t thread_id_;
std::string thread_name_;
std::unordered_set<worker::Worker*> sub_worker_contexts_;

#if HAVE_INSPECTOR
Expand Down
1 change: 1 addition & 0 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@
V(table_string, "table") \
V(target_string, "target") \
V(thread_id_string, "threadId") \
V(thread_name_string, "threadName") \
V(ticketkeycallback_string, "onticketkeycallback") \
V(timeout_string, "timeout") \
V(time_to_first_byte_string, "timeToFirstByte") \
Expand Down
10 changes: 10 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,16 @@ NODE_EXTERN Environment* CreateEnvironment(
ThreadId thread_id = {} /* allocates a thread id automatically */,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle = {});

NODE_EXTERN Environment* CreateEnvironment(
IsolateData* isolate_data,
v8::Local<v8::Context> context,
const std::vector<std::string>& args,
const std::vector<std::string>& exec_args,
EnvironmentFlags::Flags flags,
ThreadId thread_id,
std::unique_ptr<InspectorParentHandle> inspector_parent_handle,
std::string_view thread_name);

// Returns a handle that can be passed to `LoadEnvironment()`, making the
// child Environment accessible to the inspector as if it were a Node.js Worker.
// `child_thread_id` can be created using `AllocateEnvironmentThreadId()`
Expand Down
23 changes: 22 additions & 1 deletion src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ using v8::Local;
using v8::Locker;
using v8::Maybe;
using v8::Name;
using v8::NewStringType;
using v8::Null;
using v8::Number;
using v8::Object;
Expand Down Expand Up @@ -89,6 +90,15 @@ Worker::Worker(Environment* env,
Number::New(env->isolate(), static_cast<double>(thread_id_.id)))
.Check();

object()
->Set(env->context(),
env->thread_name_string(),
String::NewFromUtf8(env->isolate(),
name_.data(),
NewStringType::kNormal,
name_.size())
.ToLocalChecked())
.Check();
// Without this check, to use the permission model with
// workers (--allow-worker) one would need to pass --allow-inspector as well
if (env->permission()->is_granted(
Expand Down Expand Up @@ -365,7 +375,8 @@ void Worker::Run() {
std::move(exec_argv_),
static_cast<EnvironmentFlags::Flags>(environment_flags_),
thread_id_,
std::move(inspector_parent_handle_)));
std::move(inspector_parent_handle_),
name_));
if (is_stopped()) return;
CHECK_NOT_NULL(env_);
env_->set_env_vars(std::move(env_vars_));
Expand Down Expand Up @@ -1239,6 +1250,16 @@ void CreateWorkerPerContextProperties(Local<Object> target,
Number::New(isolate, static_cast<double>(env->thread_id())))
.Check();

target
->Set(env->context(),
env->thread_name_string(),
String::NewFromUtf8(isolate,
env->thread_name().data(),
NewStringType::kNormal,
env->thread_name().size())
.ToLocalChecked())
.Check();

target
->Set(env->context(),
FIXED_ONE_BYTE_STRING(isolate, "isMainThread"),
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-worker-thread-name.js
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) {
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the workerData?.isWorker is here only to determine if you're running in a worker or not. You can use isMainThread for that purpose and simplify this a bit.

const { Worker, isMainThread } = require('worker_threads');

if (!isMainThread) {
  // This is running in a worker
} else {
  // This is running in the main thread
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

workerData?.isWorker is designed to support the creation of worker within worker.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
Copy link
Member

Choose a reason for hiding this comment

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

There should be no reason for process.exit(0) here.

} 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);
}));
}
Loading