Skip to content

Commit

Permalink
src: do not use soon-to-be-deprecated V8 API
Browse files Browse the repository at this point in the history
V8 announced deprecation of the following methods:
 - v8::Object::SetAccessor(...) in favor of
   v8::Object::SetNativeDataProperty(...),
 - v8::ObjectTemplate::SetNativeDataProperty(...) with AccessControl
   parameter in favor of
   v8::ObjectTemplate::SetNativeDataProperty(...) without AccessControl
   parameter.

See https://crrev.com/c/5006387.

This slightly changes behavior of the following properties:
 - process.debugPort (for worker processes),
 - process.title (for worker processes),
 - process.ppid.

The difference is that they will now behave like a regular writable
JavaScript data properties - in case setter callback is not provided
they will be be reconfigured from a native data property (the one
that calls C++ callbacks upon get/set operations) to a real data
property (so subsequent reads will no longer trigger C++ getter
callbacks).

PR-URL: nodejs#53174
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
isheludko authored and bmeck committed Jun 22, 2024
1 parent 1563db4 commit f6c81ad
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 15 deletions.
29 changes: 18 additions & 11 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

namespace node {
using v8::Context;
using v8::DEFAULT;
using v8::EscapableHandleScope;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -183,13 +182,12 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {

// process.title
CHECK(process
->SetAccessor(
->SetNativeDataProperty(
context,
FIXED_ONE_BYTE_STRING(isolate, "title"),
ProcessTitleGetter,
env->owns_process_state() ? ProcessTitleSetter : nullptr,
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect)
.FromJust());
Expand All @@ -208,9 +206,15 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
READONLY_PROPERTY(process, "pid",
Integer::New(isolate, uv_os_getpid()));

CHECK(process->SetAccessor(context,
FIXED_ONE_BYTE_STRING(isolate, "ppid"),
GetParentProcessId).FromJust());
CHECK(process
->SetNativeDataProperty(context,
FIXED_ONE_BYTE_STRING(isolate, "ppid"),
GetParentProcessId,
nullptr,
Local<Value>(),
None,
SideEffectType::kHasNoSideEffect)
.FromJust());

// --security-revert flags
#define V(code, _, __) \
Expand All @@ -235,11 +239,14 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {

// process.debugPort
CHECK(process
->SetAccessor(context,
FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
DebugPortGetter,
env->owns_process_state() ? DebugPortSetter : nullptr,
Local<Value>())
->SetNativeDataProperty(
context,
FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
DebugPortGetter,
env->owns_process_state() ? DebugPortSetter : nullptr,
Local<Value>(),
None,
SideEffectType::kHasNoSideEffect)
.FromJust());
}

Expand Down
10 changes: 6 additions & 4 deletions test/parallel/test-worker-unsupported-things.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ if (!process.env.HAS_STARTED_WORKER) {
} else {
{
const before = process.title;
process.title += ' in worker';
assert.strictEqual(process.title, before);
const after = before + ' in worker';
process.title = after;
assert.strictEqual(process.title, after);
}

{
const before = process.debugPort;
process.debugPort++;
assert.strictEqual(process.debugPort, before);
const after = before + 1;
process.debugPort = after;
assert.strictEqual(process.debugPort, after);
}

{
Expand Down

0 comments on commit f6c81ad

Please sign in to comment.