FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: remove AsyncScope and AsyncCallbackScope · nodejs/node@0072a8e · GitHub

/ node Public

Commit 0072a8e

Browse files
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 e3371f0 commit 0072a8e

8 files changed

Lines changed: 37 additions & 68 deletions

File tree

‎src/api/callback.cc‎

Lines 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

‎src/async_wrap-inl.h‎

Lines 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,

‎src/async_wrap.h‎

Lines 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:

‎src/env-inl.h‎

Lines 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
}

‎src/env.h‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -721,20 +721,6 @@ class AsyncHooks : public MemoryRetainer {
721721
void grow_async_ids_stack();
722722
};
723723

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

‎src/node_http_parser.cc‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,13 @@ class Parser : public AsyncWrap, public StreamListener {
310310

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

313-
AsyncCallbackScope callback_scope(env());
314-
315-
MaybeLocal<Value> head_response =
316-
MakeCallback(cb.As<Function>(), arraysize(argv), argv);
313+
MaybeLocal<Value> head_response;
314+
{
315+
InternalCallbackScope callback_scope(
316+
this, InternalCallbackScope::kSkipTaskQueues);
317+
head_response = cb.As<Function>()->Call(
318+
env()->context(), object(), arraysize(argv), argv);
319+
}
317320

318321
int64_t val;
319322

@@ -379,9 +382,12 @@ class Parser : public AsyncWrap, public StreamListener {
379382
if (!cb->IsFunction())
380383
return 0;
381384

382-
AsyncCallbackScope callback_scope(env());
383-
384-
MaybeLocal<Value> r = MakeCallback(cb.As<Function>(), 0, nullptr);
385+
MaybeLocal<Value> r;
386+
{
387+
InternalCallbackScope callback_scope(
388+
this, InternalCallbackScope::kSkipTaskQueues);
389+
r = cb.As<Function>()->Call(env()->context(), object(), 0, nullptr);
390+
}
385391

386392
if (r.IsEmpty()) {
387393
got_exception_ = true;

‎src/node_internals.h‎

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

@@ -225,8 +231,8 @@ class InternalCallbackScope {
225231
Environment* env_;
226232
async_context async_context_;
227233
v8::Local<v8::Object> object_;
228-
AsyncCallbackScope callback_scope_;
229234
bool skip_hooks_;
235+
bool skip_task_queues_;
230236
bool failed_ = false;
231237
bool pushed_ids_ = false;
232238
bool closed_ = false;

‎src/stream_pipe.cc‎

Lines 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)
Back | FazBrowse Home | New Git URL