Skip to content

Commit

Permalink
async-wrap: don't call init callback unnecessarily
Browse files Browse the repository at this point in the history
Some calls to ReqWrap would get through the initial check and allow the
init callback to run, even though the callback had not been used on the
parent. Fix by explicitly checking if the parent has a queue.

Also change the name of the check, and internal field of AsyncHooks.
Other names were confusing.

PR-URL: nodejs#1614
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
trevnorris authored and Fishrock123 committed May 19, 2015
1 parent 6568dc9 commit a2b6650
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
8 changes: 7 additions & 1 deletion src/async-wrap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,13 @@ inline AsyncWrap::AsyncWrap(Environment* env,
// Check user controlled flag to see if the init callback should run.
if (!env->using_asyncwrap())
return;
if (!env->call_async_init_hook() && parent == nullptr)

// If callback hooks have not been enabled, and there is no parent, return.
if (!env->async_wrap_callbacks_enabled() && parent == nullptr)
return;

// If callback hooks have not been enabled and parent has no queue, return.
if (!env->async_wrap_callbacks_enabled() && !parent->has_async_queue())
return;

// TODO(trevnorris): Until it's verified all passed object's are not weak,
Expand Down
12 changes: 8 additions & 4 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,12 @@ inline int Environment::AsyncHooks::fields_count() const {
return kFieldsCount;
}

inline bool Environment::AsyncHooks::call_init_hook() {
return fields_[kCallInitHook] != 0;
inline bool Environment::AsyncHooks::callbacks_enabled() {
return fields_[kEnableCallbacks] != 0;
}

inline void Environment::AsyncHooks::set_enable_callbacks(uint32_t flag) {
fields_[kEnableCallbacks] = flag;
}

inline Environment::DomainFlag::DomainFlag() {
Expand Down Expand Up @@ -214,9 +218,9 @@ inline v8::Isolate* Environment::isolate() const {
return isolate_;
}

inline bool Environment::call_async_init_hook() const {
inline bool Environment::async_wrap_callbacks_enabled() const {
// The const_cast is okay, it doesn't violate conceptual const-ness.
return const_cast<Environment*>(this)->async_hooks()->call_init_hook();
return const_cast<Environment*>(this)->async_hooks()->callbacks_enabled();
}

inline bool Environment::in_domain() const {
Expand Down
7 changes: 4 additions & 3 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,16 @@ class Environment {
public:
inline uint32_t* fields();
inline int fields_count() const;
inline bool call_init_hook();
inline bool callbacks_enabled();
inline void set_enable_callbacks(uint32_t flag);

private:
friend class Environment; // So we can call the constructor.
inline AsyncHooks();

enum Fields {
// Set this to not zero if the init hook should be called.
kCallInitHook,
kEnableCallbacks,
kFieldsCount
};

Expand Down Expand Up @@ -374,7 +375,7 @@ class Environment {

inline v8::Isolate* isolate() const;
inline uv_loop_t* event_loop() const;
inline bool call_async_init_hook() const;
inline bool async_wrap_callbacks_enabled() const;
inline bool in_domain() const;
inline uint32_t watched_providers() const;

Expand Down

0 comments on commit a2b6650

Please sign in to comment.