Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async_wrap: make native API public #3504

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
],

'sources': [
'src/base-object.cc',
'src/debug-agent.cc',
'src/async-wrap.cc',
'src/env.cc',
Expand Down
46 changes: 0 additions & 46 deletions src/async-wrap-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,58 +4,12 @@
#include "async-wrap.h"
#include "base-object.h"
#include "base-object-inl.h"
#include "env.h"
#include "env-inl.h"
#include "node_internals.h"
#include "util.h"
#include "util-inl.h"
#include "v8.h"

namespace node {

inline AsyncWrap::AsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider,
AsyncWrap* parent)
: BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1) {
CHECK_NE(provider, PROVIDER_NONE);
CHECK_GE(object->InternalFieldCount(), 1);

// Shift provider value over to prevent id collision.
persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);

v8::Local<v8::Function> init_fn = env->async_hooks_init_function();

// No init callback exists, no reason to go on.
if (init_fn.IsEmpty())
return;

// If async wrap callbacks are disabled and no parent was passed that has
// run the init callback then return.
if (!env->async_wrap_callbacks_enabled() &&
(parent == nullptr || !parent->ran_init_callback()))
return;

v8::HandleScope scope(env->isolate());

v8::Local<v8::Value> argv[] = {
v8::Int32::New(env->isolate(), provider),
Null(env->isolate())
};

if (parent != nullptr)
argv[1] = parent->object();

v8::MaybeLocal<v8::Value> ret =
init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv);

if (ret.IsEmpty())
FatalError("node::AsyncWrap::AsyncWrap", "init hook threw");

bits_ |= 1; // ran_init_callback() is true now.
}


inline bool AsyncWrap::ran_init_callback() const {
return static_cast<bool>(bits_ & 1);
}
Expand Down
65 changes: 65 additions & 0 deletions src/async-wrap.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "async-wrap.h"
#include "async-wrap-inl.h"
#include "base-object.h"
#include "base-object-inl.h"
#include "env.h"
#include "env-inl.h"
#include "util.h"
Expand All @@ -14,9 +16,11 @@ using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::HeapProfiler;
using v8::Int32;
using v8::Integer;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Object;
using v8::RetainedObjectInfo;
using v8::TryCatch;
Expand Down Expand Up @@ -101,6 +105,67 @@ RetainedObjectInfo* WrapperInfo(uint16_t class_id, Local<Value> wrapper) {
// end RetainedAsyncInfo


void AsyncWrap::ConstructAsyncWrap(Environment* env,
Local<Object> object,
ProviderType provider,
AsyncWrap* parent) {
CHECK_NE(provider, PROVIDER_NONE);
CHECK_GE(object->InternalFieldCount(), 1);

// Shift provider value over to prevent id collision.
persistent().SetWrapperClassId(NODE_ASYNC_ID_OFFSET + provider);

Local<Function> init_fn = env->async_hooks_init_function();

// No init callback exists, no reason to go on.
if (init_fn.IsEmpty())
return;

// If async wrap callbacks are disabled and no parent was passed that has
// run the init callback then return.
if (!env->async_wrap_callbacks_enabled() &&
(parent == nullptr || !parent->ran_init_callback()))
return;

HandleScope scope(env->isolate());

Local<Value> argv[] = {
Int32::New(env->isolate(), provider),
Null(env->isolate())
};

if (parent != nullptr)
argv[1] = parent->object();

MaybeLocal<Value> ret =
init_fn->Call(env->context(), object, ARRAY_SIZE(argv), argv);

if (ret.IsEmpty())
FatalError("node::AsyncWrap::AsyncWrap", "init hook threw");

bits_ |= 1; // ran_init_callback() is true now.
}


AsyncWrap::AsyncWrap(Environment* env,
Local<Object> object,
ProviderType provider,
AsyncWrap* parent)
: BaseObject(env, object), bits_(static_cast<uint32_t>(provider) << 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make bits_ a uintptr_t so there is room to change it to a pointer without breaking ABI.

ConstructAsyncWrap(env, object, provider, parent);
}


AsyncWrap::AsyncWrap(Isolate* isolate,
Local<Object> object,
ProviderType provider,
AsyncWrap* parent)
: BaseObject(isolate, object), bits_(static_cast<uint32_t>(provider) << 1) {
Environment* env = Environment::GetCurrent(isolate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly!

ConstructAsyncWrap(env, object, provider, parent);
}


static void EnableHooksJS(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
env->async_hooks()->set_enable_callbacks(1);
Expand Down
25 changes: 19 additions & 6 deletions src/async-wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ namespace node {
V(UDPWRAP) \
V(UDPSENDWRAP) \
V(WRITEWRAP) \
V(ZLIB)
V(ZLIB) \
V(USER)

class Environment;

Expand All @@ -46,10 +47,16 @@ class AsyncWrap : public BaseObject {
#undef V
};

inline AsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider,
AsyncWrap* parent = nullptr);
AsyncWrap(v8::Isolate* isolate,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to make this header public then you should make all methods non-inline. Otherwise it may get hard to maintain ABI stability.

v8::Local<v8::Object> object,
ProviderType provider,
AsyncWrap* parent = nullptr);

// Private API. Users don't have access to Environment.
AsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider,
AsyncWrap* parent = nullptr);

inline virtual ~AsyncWrap() override = default;

Expand All @@ -66,11 +73,17 @@ class AsyncWrap : public BaseObject {
int argc,
v8::Local<v8::Value>* argv);

inline bool ran_init_callback() const;

virtual size_t self_size() const = 0;

private:
inline AsyncWrap();
inline bool ran_init_callback() const;

void ConstructAsyncWrap(Environment* env,
v8::Local<v8::Object> object,
ProviderType provider,
AsyncWrap* parent);

// When the async hooks init JS function is called from the constructor it is
// expected the context object will receive a _asyncQueue object property
Expand Down
20 changes: 11 additions & 9 deletions src/base-object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,18 @@
#define SRC_BASE_OBJECT_INL_H_

#include "base-object.h"
#include "env.h"
#include "env-inl.h"
#include "util.h"
#include "util-inl.h"
#include "v8.h"

namespace node {

inline BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> handle)
: handle_(env->isolate(), handle),
env_(env) {
CHECK_EQ(false, handle.IsEmpty());
inline BaseObject::BaseObject(v8::Isolate* isolate,
v8::Local<v8::Object> handle)
: handle_(isolate, handle),
isolate_(isolate) {
}


inline BaseObject::~BaseObject() {
CHECK(handle_.IsEmpty());
}
Expand All @@ -28,7 +25,12 @@ inline v8::Persistent<v8::Object>& BaseObject::persistent() {


inline v8::Local<v8::Object> BaseObject::object() {
return PersistentToLocal(env_->isolate(), handle_);
return PersistentToLocal(isolate(), handle_);
}


inline v8::Isolate* BaseObject::isolate() const {
return isolate_;
}


Expand All @@ -48,7 +50,7 @@ inline void BaseObject::WeakCallback(

template <typename Type>
inline void BaseObject::MakeWeak(Type* ptr) {
v8::HandleScope scope(env_->isolate());
v8::HandleScope scope(isolate());
v8::Local<v8::Object> handle = object();
CHECK_GT(handle->InternalFieldCount(), 0);
Wrap(handle, ptr);
Expand Down
18 changes: 18 additions & 0 deletions src/base-object.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#include "base-object.h"
#include "base-object-inl.h"
#include "env.h"
#include "env-inl.h"

namespace node {

using v8::Local;
using v8::Object;

BaseObject::BaseObject(Environment* env, Local<Object> handle)
: handle_(env->isolate(), handle),
isolate_(env->isolate()),
env_(env) {
CHECK_EQ(false, handle.IsEmpty());
}

} // namespace node
4 changes: 4 additions & 0 deletions src/base-object.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class Environment;
class BaseObject {
public:
BaseObject(Environment* env, v8::Local<v8::Object> handle);
BaseObject(v8::Isolate* isolate, v8::Local<v8::Object> handle);
virtual ~BaseObject();

// Returns the wrapped object. Returns an empty handle when
Expand All @@ -23,6 +24,8 @@ class BaseObject {
// calling .Reset() again is harmless.
inline v8::Persistent<v8::Object>& persistent();

inline v8::Isolate* isolate() const;

inline Environment* env() const;

// The handle_ must have an internal field count > 0, and the first
Expand All @@ -43,6 +46,7 @@ class BaseObject {
const v8::WeakCallbackData<v8::Object, Type>& data);

v8::Persistent<v8::Object> handle_;
v8::Isolate* isolate_;
Environment* env_;
};

Expand Down
6 changes: 2 additions & 4 deletions test/parallel/test-async-wrap-check-providers.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ const StreamWrap = require('_stream_wrap').StreamWrap;
const async_wrap = process.binding('async_wrap');
const pkeys = Object.keys(async_wrap.Providers);

let keyList = pkeys.slice();
// Drop NONE
keyList.splice(0, 1);

// Remove the 'NONE' and 'USER' providers.
let keyList = pkeys.slice().filter(e => e != 'NONE' && e != 'USER');

function init(id) {
keyList = keyList.filter(e => e != pkeys[id]);
Expand Down