Skip to content

Commit 3509d74

Browse files
addaleaxaddaleax
committed
src: refactor Environment::GetCurrent(isolate) usage
Do not require an explicit `HandleScope`, or the ability to create one, when using `Environment::GetCurrent()`. `isolate->InContext()` is used as an indicator that it is probably okay to create a `HandleScope`, see also the short discussion in nodejs#25775 (review).
1 parent 410eb97 commit 3509d74

5 files changed

Lines changed: 13 additions & 9 deletions

File tree

‎src/api/environment.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ using v8::MaybeLocal;
2323
using v8::Message;
2424
using v8::MicrotasksPolicy;
2525
using v8::ObjectTemplate;
26+
using v8::SealHandleScope;
2627
using v8::String;
2728
using v8::Value;
2829

@@ -34,7 +35,9 @@ static bool AllowWasmCodeGenerationCallback(Local<Context> context,
3435
}
3536

3637
static bool ShouldAbortOnUncaughtException(Isolate* isolate) {
37-
HandleScope scope(isolate);
38+
#ifdef DEBUG
39+
SealHandleScope scope(isolate);
40+
#endif
3841
Environment* env = Environment::GetCurrent(isolate);
3942
return env != nullptr &&
4043
(env->is_main_thread() || !env->is_stopping_worker()) &&

‎src/api/exceptions.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Local<Value> ErrnoException(Isolate* isolate,
2828
const char* msg,
2929
const char* path) {
3030
Environment* env = Environment::GetCurrent(isolate);
31+
CHECK_NOT_NULL(env);
3132

3233
Local<Value> e;
3334
Local<String> estring = OneByteString(isolate, errors::errno_string(errorno));
@@ -99,6 +100,7 @@ Local<Value> UVException(Isolate* isolate,
99100
const char* path,
100101
const char* dest) {
101102
Environment* env = Environment::GetCurrent(isolate);
103+
CHECK_NOT_NULL(env);
102104

103105
if (!msg || !msg[0])
104106
msg = uv_strerror(errorno);
@@ -187,6 +189,7 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
187189
const char* msg,
188190
const char* path) {
189191
Environment* env = Environment::GetCurrent(isolate);
192+
CHECK_NOT_NULL(env);
190193
Local<Value> e;
191194
bool must_free = false;
192195
if (!msg || !msg[0]) {

‎src/api/hooks.cc‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ using v8::Integer;
1111
using v8::Isolate;
1212
using v8::Local;
1313
using v8::Object;
14+
using v8::SealHandleScope;
1415
using v8::String;
1516
using v8::Value;
1617
using v8::NewStringType;
@@ -88,16 +89,12 @@ void RemoveEnvironmentCleanupHook(Isolate* isolate,
8889
}
8990

9091
async_id AsyncHooksGetExecutionAsyncId(Isolate* isolate) {
91-
// Environment::GetCurrent() allocates a Local<> handle.
92-
HandleScope handle_scope(isolate);
9392
Environment* env = Environment::GetCurrent(isolate);
9493
if (env == nullptr) return -1;
9594
return env->execution_async_id();
9695
}
9796

9897
async_id AsyncHooksGetTriggerAsyncId(Isolate* isolate) {
99-
// Environment::GetCurrent() allocates a Local<> handle.
100-
HandleScope handle_scope(isolate);
10198
Environment* env = Environment::GetCurrent(isolate);
10299
if (env == nullptr) return -1;
103100
return env->trigger_async_id();
@@ -119,7 +116,9 @@ async_context EmitAsyncInit(Isolate* isolate,
119116
Local<Object> resource,
120117
Local<String> name,
121118
async_id trigger_async_id) {
122-
HandleScope handle_scope(isolate);
119+
#ifdef DEBUG
120+
SealHandleScope handle_scope(isolate);
121+
#endif
123122
Environment* env = Environment::GetCurrent(isolate);
124123
CHECK_NOT_NULL(env);
125124

@@ -140,8 +139,6 @@ async_context EmitAsyncInit(Isolate* isolate,
140139
}
141140

142141
void EmitAsyncDestroy(Isolate* isolate, async_context asyncContext) {
143-
// Environment::GetCurrent() allocates a Local<> handle.
144-
HandleScope handle_scope(isolate);
145142
AsyncWrap::EmitDestroy(
146143
Environment::GetCurrent(isolate), asyncContext.async_id);
147144
}

‎src/env-inl.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,8 @@ inline void Environment::AssignToContext(v8::Local<v8::Context> context,
296296
}
297297

298298
inline Environment* Environment::GetCurrent(v8::Isolate* isolate) {
299+
if (UNLIKELY(!isolate->InContext())) return nullptr;
300+
v8::HandleScope handle_scope(isolate);
299301
return GetCurrent(isolate->GetCurrentContext());
300302
}
301303

‎src/node_errors.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,6 @@ void OnFatalError(const char* location, const char* message) {
314314
}
315315
#ifdef NODE_REPORT
316316
Isolate* isolate = Isolate::GetCurrent();
317-
HandleScope handle_scope(isolate);
318317
Environment* env = Environment::GetCurrent(isolate);
319318
if (env == nullptr || env->isolate_data()->options()->report_on_fatalerror) {
320319
report::TriggerNodeReport(

0 commit comments

Comments
 (0)