Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f080239

Browse filesBrowse files
addaleaxMylesBorins
addaleax
authored and
MylesBorins
committed
src: remove AsyncScope and AsyncCallbackScope
Reduce the number of different scopes we use for async callbacks. PR-URL: #30236 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f70d518 commit f080239
Copy full SHA for f080239

File tree

Expand file treeCollapse file tree

8 files changed

+37
-68
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+37
-68
lines changed
Open diff view settings
Collapse file

‎src/api/callback.cc‎

Copy file name to clipboardExpand all lines: src/api/callback.cc
+8-5Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,12 @@ CallbackScope::~CallbackScope() {
3434
delete private_;
3535
}
3636

37-
InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap)
37+
InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap, int flags)
3838
: InternalCallbackScope(async_wrap->env(),
3939
async_wrap->object(),
4040
{ async_wrap->get_async_id(),
41-
async_wrap->get_trigger_async_id() }) {}
41+
async_wrap->get_trigger_async_id() },
42+
flags) {}
4243

4344
InternalCallbackScope::InternalCallbackScope(Environment* env,
4445
Local<Object> object,
@@ -47,10 +48,11 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
4748
: env_(env),
4849
async_context_(asyncContext),
4950
object_(object),
50-
callback_scope_(env),
51-
skip_hooks_(flags & kSkipAsyncHooks) {
51+
skip_hooks_(flags & kSkipAsyncHooks),
52+
skip_task_queues_(flags & kSkipTaskQueues) {
5253
CHECK_IMPLIES(!(flags & kAllowEmptyResource), !object.IsEmpty());
5354
CHECK_NOT_NULL(env);
55+
env->PushAsyncCallbackScope();
5456

5557
if (!env->can_call_into_js()) {
5658
failed_ = true;
@@ -74,6 +76,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
7476

7577
InternalCallbackScope::~InternalCallbackScope() {
7678
Close();
79+
env_->PopAsyncCallbackScope();
7780
}
7881

7982
void InternalCallbackScope::Close() {
@@ -94,7 +97,7 @@ void InternalCallbackScope::Close() {
9497
AsyncWrap::EmitAfter(env_, async_context_.async_id);
9598
}
9699

97-
if (env_->async_callback_scope_depth() > 1) {
100+
if (env_->async_callback_scope_depth() > 1 || skip_task_queues_) {
98101
return;
99102
}
100103

Collapse file

‎src/async_wrap-inl.h‎

Copy file name to clipboardExpand all lines: src/async_wrap-inl.h
-14Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,20 +50,6 @@ inline double AsyncWrap::get_trigger_async_id() const {
5050
}
5151

5252

53-
inline AsyncWrap::AsyncScope::AsyncScope(AsyncWrap* wrap)
54-
: wrap_(wrap) {
55-
Environment* env = wrap->env();
56-
if (env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) return;
57-
EmitBefore(env, wrap->get_async_id());
58-
}
59-
60-
inline AsyncWrap::AsyncScope::~AsyncScope() {
61-
Environment* env = wrap_->env();
62-
if (env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) return;
63-
EmitAfter(env, wrap_->get_async_id());
64-
}
65-
66-
6753
inline v8::MaybeLocal<v8::Value> AsyncWrap::MakeCallback(
6854
const v8::Local<v8::String> symbol,
6955
int argc,
Collapse file

‎src/async_wrap.h‎

Copy file name to clipboardExpand all lines: src/async_wrap.h
-13Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,6 @@ class AsyncWrap : public BaseObject {
162162
inline ProviderType set_provider_type(ProviderType provider);
163163

164164
inline double get_async_id() const;
165-
166165
inline double get_trigger_async_id() const;
167166

168167
void AsyncReset(v8::Local<v8::Object> resource,
@@ -200,18 +199,6 @@ class AsyncWrap : public BaseObject {
200199
static v8::Local<v8::Object> GetOwner(Environment* env,
201200
v8::Local<v8::Object> obj);
202201

203-
// This is a simplified version of InternalCallbackScope that only runs
204-
// the `before` and `after` hooks. Only use it when not actually calling
205-
// back into JS; otherwise, use InternalCallbackScope.
206-
class AsyncScope {
207-
public:
208-
explicit inline AsyncScope(AsyncWrap* wrap);
209-
~AsyncScope();
210-
211-
private:
212-
AsyncWrap* wrap_ = nullptr;
213-
};
214-
215202
bool IsDoneInitializing() const override;
216203

217204
private:
Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
-8Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,6 @@ Environment* Environment::ForAsyncHooks(AsyncHooks* hooks) {
211211
return ContainerOf(&Environment::async_hooks_, hooks);
212212
}
213213

214-
inline AsyncCallbackScope::AsyncCallbackScope(Environment* env) : env_(env) {
215-
env_->PushAsyncCallbackScope();
216-
}
217-
218-
inline AsyncCallbackScope::~AsyncCallbackScope() {
219-
env_->PopAsyncCallbackScope();
220-
}
221-
222214
inline size_t Environment::async_callback_scope_depth() const {
223215
return async_callback_scope_depth_;
224216
}
Collapse file

‎src/env.h‎

Copy file name to clipboardExpand all lines: src/env.h
-14Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -723,20 +723,6 @@ class AsyncHooks : public MemoryRetainer {
723723
void grow_async_ids_stack();
724724
};
725725

726-
class AsyncCallbackScope {
727-
public:
728-
AsyncCallbackScope() = delete;
729-
explicit AsyncCallbackScope(Environment* env);
730-
~AsyncCallbackScope();
731-
AsyncCallbackScope(const AsyncCallbackScope&) = delete;
732-
AsyncCallbackScope& operator=(const AsyncCallbackScope&) = delete;
733-
AsyncCallbackScope(AsyncCallbackScope&&) = delete;
734-
AsyncCallbackScope& operator=(AsyncCallbackScope&&) = delete;
735-
736-
private:
737-
Environment* env_;
738-
};
739-
740726
class ImmediateInfo : public MemoryRetainer {
741727
public:
742728
inline AliasedUint32Array& fields();
Collapse file

‎src/node_http_parser_impl.h‎

Copy file name to clipboardExpand all lines: src/node_http_parser_impl.h
+13-7Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,10 +321,13 @@ class Parser : public AsyncWrap, public StreamListener {
321321

322322
argv[A_UPGRADE] = Boolean::New(env()->isolate(), parser_.upgrade);
323323

324-
AsyncCallbackScope callback_scope(env());
325-
326-
MaybeLocal<Value> head_response =
327-
MakeCallback(cb.As<Function>(), arraysize(argv), argv);
324+
MaybeLocal<Value> head_response;
325+
{
326+
InternalCallbackScope callback_scope(
327+
this, InternalCallbackScope::kSkipTaskQueues);
328+
head_response = cb.As<Function>()->Call(
329+
env()->context(), object(), arraysize(argv), argv);
330+
}
328331

329332
int64_t val;
330333

@@ -392,9 +395,12 @@ class Parser : public AsyncWrap, public StreamListener {
392395
if (!cb->IsFunction())
393396
return 0;
394397

395-
AsyncCallbackScope callback_scope(env());
396-
397-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
398+
MaybeLocal<Value> r;
399+
{
400+
InternalCallbackScope callback_scope(
401+
this, InternalCallbackScope::kSkipTaskQueues);
402+
r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
403+
}
398404

399405
if (r.IsEmpty()) {
400406
got_exception_ = true;
Collapse file

‎src/node_internals.h‎

Copy file name to clipboardExpand all lines: src/node_internals.h
+10-4Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -206,17 +206,23 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
206206
class InternalCallbackScope {
207207
public:
208208
enum Flags {
209+
kNoFlags = 0,
209210
// Tell the constructor whether its `object` parameter may be empty or not.
210211
kAllowEmptyResource = 1,
211212
// Indicates whether 'before' and 'after' hooks should be skipped.
212-
kSkipAsyncHooks = 2
213+
kSkipAsyncHooks = 2,
214+
// Indicates whether nextTick and microtask queues should be skipped.
215+
// This should only be used when there is no call into JS in this scope.
216+
// (The HTTP parser also uses it for some weird backwards
217+
// compatibility issues, but it shouldn't.)
218+
kSkipTaskQueues = 4
213219
};
214220
InternalCallbackScope(Environment* env,
215221
v8::Local<v8::Object> object,
216222
const async_context& asyncContext,
217-
int flags = 0);
223+
int flags = kNoFlags);
218224
// Utility that can be used by AsyncWrap classes.
219-
explicit InternalCallbackScope(AsyncWrap* async_wrap);
225+
explicit InternalCallbackScope(AsyncWrap* async_wrap, int flags = 0);
220226
~InternalCallbackScope();
221227
void Close();
222228

@@ -227,8 +233,8 @@ class InternalCallbackScope {
227233
Environment* env_;
228234
async_context async_context_;
229235
v8::Local<v8::Object> object_;
230-
AsyncCallbackScope callback_scope_;
231236
bool skip_hooks_;
237+
bool skip_task_queues_;
232238
bool failed_ = false;
233239
bool pushed_ids_ = false;
234240
bool closed_ = false;
Collapse file

‎src/stream_pipe.cc‎

Copy file name to clipboardExpand all lines: src/stream_pipe.cc
+6-3Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ void StreamPipe::ReadableListener::OnStreamRead(ssize_t nread,
119119
const uv_buf_t& buf_) {
120120
StreamPipe* pipe = ContainerOf(&StreamPipe::readable_listener_, this);
121121
AllocatedBuffer buf(pipe->env(), buf_);
122-
AsyncScope async_scope(pipe);
123122
if (nread < 0) {
124123
// EOF or error; stop reading and pass the error to the previous listener
125124
// (which might end up in JS).
@@ -162,7 +161,9 @@ void StreamPipe::WritableListener::OnStreamAfterWrite(WriteWrap* w,
162161
StreamPipe* pipe = ContainerOf(&StreamPipe::writable_listener_, this);
163162
pipe->is_writing_ = false;
164163
if (pipe->is_eof_) {
165-
AsyncScope async_scope(pipe);
164+
HandleScope handle_scope(pipe->env()->isolate());
165+
InternalCallbackScope callback_scope(pipe,
166+
InternalCallbackScope::kSkipTaskQueues);
166167
pipe->ShutdownWritable();
167168
pipe->Unpipe();
168169
return;
@@ -206,7 +207,9 @@ void StreamPipe::WritableListener::OnStreamWantsWrite(size_t suggested_size) {
206207
pipe->wanted_data_ = suggested_size;
207208
if (pipe->is_reading_ || pipe->is_closed_)
208209
return;
209-
AsyncScope async_scope(pipe);
210+
HandleScope handle_scope(pipe->env()->isolate());
211+
InternalCallbackScope callback_scope(pipe,
212+
InternalCallbackScope::kSkipTaskQueues);
210213
pipe->is_reading_ = true;
211214
pipe->source()->ReadStart();
212215
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.