Skip to content

Commit

Permalink
fixup! squash! src: retrieve binding data from the context
Browse files Browse the repository at this point in the history
  • Loading branch information
addaleax committed May 5, 2020
1 parent a3e1371 commit 2ff3173
Show file tree
Hide file tree
Showing 20 changed files with 127 additions and 111 deletions.
21 changes: 13 additions & 8 deletions src/README.md
Expand Up @@ -400,16 +400,22 @@ NODE_MODULE_CONTEXT_AWARE_INTERNAL(cares_wrap, Initialize)
Some internal bindings, such as the HTTP parser, maintain internal state that
only affects that particular binding. In that case, one common way to store
that state is through the use of `Environment::BindingScope`, which gives all
new functions created within it access to an object for storing such state.
that state is through the use of `Environment::AddBindingData`, which gives
binding functions access to an object for storing such state.
That object is always a [`BaseObject`][].
Its class needs to have a static `binding_data_name` field that based on a
constant string, in order to disambiguate it from other classes of this type,
and which could e.g. match the binding’s name.
```c++
// In the HTTP parser source code file:
class BindingData : public BaseObject {
public:
BindingData(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
static constexpr FastStringKey binding_data_name { "http_parser" };
std::vector<char> parser_buffer;
bool parser_buffer_in_use = false;
Expand All @@ -422,18 +428,17 @@ static void New(const FunctionCallbackInfo<Value>& args) {
new Parser(binding_data, args.This());
}
// ... because the initialization function told the Environment to use this
// BindingData class for all functions created by it:
// ... because the initialization function told the Environment to store the
// BindingData object:
void InitializeHttpParser(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Environment::BindingScope<BindingData> binding_scope(context, target);
if (!binding_scope) return;
BindingData* binding_data = binding_scope.data;
BindingData* const binding_data =
env->AddBindingData<BindingData>(context, target);
if (binding_data == nullptr) return;
// Created within the Environment::BindingScope
Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
...
}
Expand Down
50 changes: 13 additions & 37 deletions src/env-inl.h
Expand Up @@ -333,58 +333,43 @@ inline Environment* Environment::GetCurrent(

template <typename T, typename U>
inline T* Environment::GetBindingData(const v8::PropertyCallbackInfo<U>& info) {
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
}

template <typename T>
inline T* Environment::GetBindingData(
const v8::FunctionCallbackInfo<v8::Value>& info) {
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext(), info.Data());
return GetBindingData<T>(info.GetIsolate()->GetCurrentContext());
}

template <typename T>
inline T* Environment::GetBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Value> val) {
DCHECK(val->IsUint32());
uint32_t index = val.As<v8::Uint32>()->Value();
BindingDataList* list = static_cast<BindingDataList*>(
inline T* Environment::GetBindingData(v8::Local<v8::Context> context) {
BindingDataStore* list = static_cast<BindingDataStore*>(
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingListIndex));
DCHECK_NOT_NULL(list);
DCHECK_GT(list->size(), index);
T* result = static_cast<T*>(list->at(index).get());
auto it = list->find(T::binding_data_name);
DCHECK_NE(it, list->end());
T* result = static_cast<T*>(it->second.get());
DCHECK_NOT_NULL(result);
DCHECK_EQ(result->env(), GetCurrent(context));
return result;
}

template <typename T>
inline std::pair<T*, uint32_t> Environment::NewBindingData(
inline T* Environment::AddBindingData(
v8::Local<v8::Context> context,
v8::Local<v8::Object> target) {
DCHECK_EQ(GetCurrent(context), this);
// This won't compile if T is not a BaseObject subclass.
BaseObjectPtr<T> item = MakeDetachedBaseObject<T>(this, target);
BindingDataList* list = static_cast<BindingDataList*>(
BindingDataStore* list = static_cast<BindingDataStore*>(
context->GetAlignedPointerFromEmbedderData(
ContextEmbedderIndex::kBindingListIndex));
DCHECK_NOT_NULL(list);
size_t index = list->size();
list->emplace_back(item);
return std::make_pair(item.get(), index);
}

template <typename T>
Environment::BindingScope<T>::BindingScope(v8::Local<v8::Context> context,
v8::Local<v8::Object> target)
: env(Environment::GetCurrent(context)) {
std::tie(data, env->current_callback_data_) =
env->NewBindingData<T>(context, target);
}

template <typename T>
Environment::BindingScope<T>::~BindingScope() {
env->current_callback_data_ = env->default_callback_data_;
auto result = list->emplace(T::binding_data_name, item);
CHECK(result.second);
return item.get();
}

inline Environment* Environment::GetThreadLocalEnv() {
Expand Down Expand Up @@ -1103,8 +1088,7 @@ inline v8::Local<v8::FunctionTemplate>
v8::Local<v8::Signature> signature,
v8::ConstructorBehavior behavior,
v8::SideEffectType side_effect_type) {
v8::Local<v8::Value> external = current_callback_data();
return v8::FunctionTemplate::New(isolate(), callback, external,
return v8::FunctionTemplate::New(isolate(), callback, v8::Local<v8::Value>(),
signature, 0, behavior, side_effect_type);
}

Expand Down Expand Up @@ -1300,14 +1284,6 @@ v8::Local<v8::Context> Environment::context() const {
return PersistentToLocal::Strong(context_);
}

v8::Local<v8::Value> Environment::as_callback_data() const {
return v8::Integer::NewFromUnsigned(isolate(), default_callback_data_);
}

v8::Local<v8::Value> Environment::current_callback_data() const {
return v8::Integer::NewFromUnsigned(isolate(), current_callback_data_);
}

} // namespace node

// These two files depend on each other. Including base_object-inl.h after this
Expand Down
15 changes: 0 additions & 15 deletions src/env.cc
Expand Up @@ -261,16 +261,6 @@ void TrackingTraceStateObserver::UpdateTraceCategoryState() {
USE(cb->Call(env_->context(), Undefined(isolate), arraysize(args), args));
}

class NoBindingData : public BaseObject {
public:
NoBindingData(Environment* env, Local<Object> obj)
: BaseObject(env, obj) {}

SET_NO_MEMORY_INFO()
SET_MEMORY_INFO_NAME(NoBindingData)
SET_SELF_SIZE(NoBindingData)
};

void Environment::CreateProperties() {
HandleScope handle_scope(isolate_);
Local<Context> ctx = context();
Expand All @@ -282,11 +272,6 @@ void Environment::CreateProperties() {
BaseObject::kInternalFieldCount);

set_binding_data_ctor_template(templ);
Local<Function> ctor = templ->GetFunction(ctx).ToLocalChecked();
Local<Object> obj = ctor->NewInstance(ctx).ToLocalChecked();
uint32_t index = NewBindingData<NoBindingData>(ctx, obj).second;
default_callback_data_ = index;
current_callback_data_ = index;
}

// Store primordials setup by the per-context script in the environment.
Expand Down
39 changes: 9 additions & 30 deletions src/env.h
Expand Up @@ -864,38 +864,22 @@ class Environment : public MemoryRetainer {

// Methods created using SetMethod(), SetPrototypeMethod(), etc. inside
// this scope can access the created T* object using
// GetBindingData<T>(args.Data()) later.
// GetBindingData<T>(args) later.
template <typename T>
struct BindingScope {
explicit inline BindingScope(v8::Local<v8::Context> context,
v8::Local<v8::Object> target);
inline ~BindingScope();

T* data = nullptr;
Environment* env;

inline operator bool() const { return data != nullptr; }
inline bool operator !() const { return data == nullptr; }
};

T* AddBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Object> target);
template <typename T, typename U>
static inline T* GetBindingData(const v8::PropertyCallbackInfo<U>& info);
template <typename T>
static inline T* GetBindingData(
const v8::FunctionCallbackInfo<v8::Value>& info);

// Unwrap a subclass T object from a v8::Value which needs to be an
// v8::Uint32
template <typename T>
static inline T* GetBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Value> val);
// Create a BindingData of subclass T, put it into the context binding list,
// return the index as an integer
template <typename T>
inline std::pair<T*, uint32_t> NewBindingData(v8::Local<v8::Context> context,
v8::Local<v8::Object> target);
static inline T* GetBindingData(v8::Local<v8::Context> context);

typedef std::vector<BaseObjectPtr<BaseObject>> BindingDataList;
typedef std::unordered_map<
FastStringKey,
BaseObjectPtr<BaseObject>,
FastStringKey::Hash> BindingDataStore;

static uv_key_t thread_local_env;
static inline Environment* GetThreadLocalEnv();
Expand Down Expand Up @@ -1146,8 +1130,6 @@ class Environment : public MemoryRetainer {
#undef V

inline v8::Local<v8::Context> context() const;
inline v8::Local<v8::Value> as_callback_data() const;
inline v8::Local<v8::Value> current_callback_data() const;

#if HAVE_INSPECTOR
inline inspector::Agent* inspector_agent() const {
Expand Down Expand Up @@ -1443,7 +1425,7 @@ class Environment : public MemoryRetainer {
void RequestInterruptFromV8();
static void CheckImmediate(uv_check_t* handle);

BindingDataList bindings_;
BindingDataStore bindings_;

// Use an unordered_set, so that we have efficient insertion and removal.
std::unordered_set<CleanupHookCallback,
Expand All @@ -1469,9 +1451,6 @@ class Environment : public MemoryRetainer {

v8::Global<v8::Context> context_;

uint32_t default_callback_data_;
uint32_t current_callback_data_;

// Keeps the main script source alive is one was passed to LoadEnvironment().
// We should probably find a way to just use plain `v8::String`s created from
// the source passed to LoadEnvironment() directly instead.
Expand Down
2 changes: 1 addition & 1 deletion src/fs_event_wrap.cc
Expand Up @@ -108,7 +108,7 @@ void FSEventWrap::Initialize(Local<Object> target,
Local<FunctionTemplate> get_initialized_templ =
FunctionTemplate::New(env->isolate(),
GetInitialized,
env->current_callback_data(),
Local<Value>(),
Signature::New(env->isolate(), t));

t->PrototypeTemplate()->SetAccessorProperty(
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Expand Up @@ -331,7 +331,7 @@ MaybeLocal<Value> Environment::BootstrapNode() {

Local<String> env_string = FIXED_ONE_BYTE_STRING(isolate_, "env");
Local<Object> env_var_proxy;
if (!CreateEnvVarProxy(context(), isolate_, current_callback_data())
if (!CreateEnvVarProxy(context(), isolate_, Local<Value>())
.ToLocal(&env_var_proxy) ||
process_object()->Set(context(), env_string, env_var_proxy).IsNothing()) {
return MaybeLocal<Value>();
Expand Down
4 changes: 2 additions & 2 deletions src/node_crypto.cc
Expand Up @@ -505,7 +505,7 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
Local<FunctionTemplate> ctx_getter_templ =
FunctionTemplate::New(env->isolate(),
CtxGetter,
env->current_callback_data(),
Local<Value>(),
Signature::New(env->isolate(), t));


Expand Down Expand Up @@ -5103,7 +5103,7 @@ void DiffieHellman::Initialize(Environment* env, Local<Object> target) {
Local<FunctionTemplate> verify_error_getter_templ =
FunctionTemplate::New(env->isolate(),
DiffieHellman::VerifyErrorGetter,
env->current_callback_data(),
Local<Value>(),
Signature::New(env->isolate(), t),
/* length */ 0,
ConstructorBehavior::kThrow,
Expand Down
9 changes: 6 additions & 3 deletions src/node_file.cc
Expand Up @@ -2303,15 +2303,18 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const {
file_handle_read_wrap_freelist);
}

// TODO(addaleax): Remove once we're on C++17.
constexpr FastStringKey BindingData::binding_data_name;

void Initialize(Local<Object> target,
Local<Value> unused,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();
Environment::BindingScope<BindingData> binding_scope(context, target);
if (!binding_scope) return;
BindingData* binding_data = binding_scope.data;
BindingData* const binding_data =
env->AddBindingData<BindingData>(context, target);
if (binding_data == nullptr) return;

env->SetMethod(target, "access", Access);
env->SetMethod(target, "close", Close);
Expand Down
2 changes: 2 additions & 0 deletions src/node_file.h
Expand Up @@ -26,6 +26,8 @@ class BindingData : public BaseObject {
std::vector<BaseObjectPtr<FileHandleReadWrap>>
file_handle_read_wrap_freelist;

static constexpr FastStringKey binding_data_name { "fs" };

void MemoryInfo(MemoryTracker* tracker) const override;
SET_SELF_SIZE(BindingData)
SET_MEMORY_INFO_NAME(BindingData)
Expand Down
8 changes: 5 additions & 3 deletions src/node_http2.cc
Expand Up @@ -2941,6 +2941,9 @@ void Http2State::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("root_buffer", root_buffer);
}

// TODO(addaleax): Remove once we're on C++17.
constexpr FastStringKey Http2State::binding_data_name;

// Set up the process.binding('http2') binding.
void Initialize(Local<Object> target,
Local<Value> unused,
Expand All @@ -2950,9 +2953,8 @@ void Initialize(Local<Object> target,
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);

Environment::BindingScope<Http2State> binding_scope(context, target);
if (!binding_scope) return;
Http2State* state = binding_scope.data;
Http2State* const state = env->AddBindingData<Http2State>(context, target);
if (state == nullptr) return;

#define SET_STATE_TYPEDARRAY(name, field) \
target->Set(context, \
Expand Down
2 changes: 2 additions & 0 deletions src/node_http2_state.h
Expand Up @@ -126,6 +126,8 @@ class Http2State : public BaseObject {
SET_SELF_SIZE(Http2State)
SET_MEMORY_INFO_NAME(Http2State)

static constexpr FastStringKey binding_data_name { "http2" };

private:
struct http2_state_internal {
// doubles first so that they are always sizeof(double)-aligned
Expand Down
10 changes: 8 additions & 2 deletions src/node_http_parser.cc
Expand Up @@ -87,6 +87,8 @@ class BindingData : public BaseObject {
BindingData(Environment* env, Local<Object> obj)
: BaseObject(env, obj) {}

static constexpr FastStringKey binding_data_name { "http_parser" };

std::vector<char> parser_buffer;
bool parser_buffer_in_use = false;

Expand All @@ -97,6 +99,9 @@ class BindingData : public BaseObject {
SET_MEMORY_INFO_NAME(BindingData)
};

// TODO(addaleax): Remove once we're on C++17.
constexpr FastStringKey BindingData::binding_data_name;

// helper class for the Parser
struct StringPtr {
StringPtr() {
Expand Down Expand Up @@ -921,8 +926,9 @@ void InitializeHttpParser(Local<Object> target,
Local<Context> context,
void* priv) {
Environment* env = Environment::GetCurrent(context);
Environment::BindingScope<BindingData> binding_scope(context, target);
if (!binding_scope) return;
BindingData* const binding_data =
env->AddBindingData<BindingData>(context, target);
if (binding_data == nullptr) return;

Local<FunctionTemplate> t = env->NewFunctionTemplate(Parser::New);
t->InstanceTemplate()->SetInternalFieldCount(Parser::kInternalFieldCount);
Expand Down
2 changes: 1 addition & 1 deletion src/node_native_module_env.cc
Expand Up @@ -190,7 +190,7 @@ void NativeModuleEnv::Initialize(Local<Object> target,
FIXED_ONE_BYTE_STRING(env->isolate(), "moduleCategories"),
GetModuleCategories,
nullptr,
env->as_callback_data(),
Local<Value>(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect)
Expand Down

0 comments on commit 2ff3173

Please sign in to comment.