diff --git a/src/debug-agent.cc b/src/debug-agent.cc index e4e0ea061bd30f..78eacf49ab2fc8 100644 --- a/src/debug-agent.cc +++ b/src/debug-agent.cc @@ -172,9 +172,15 @@ void Agent::WorkerRun() { Local context = Context::New(isolate); Context::Scope context_scope(context); + + // FIXME(bnoordhuis) Work around V8 bug: v8::Private::ForApi() dereferences + // a nullptr when a context hasn't been entered first. The symbol registry + // is a lazily created JS object but object instantiation does not work + // without a context. + IsolateData isolate_data(isolate, &child_loop_); + Environment* env = CreateEnvironment( - isolate, - &child_loop_, + &isolate_data, context, arraysize(argv), argv, diff --git a/src/env-inl.h b/src/env-inl.h index a8a7d0255211b0..cf62ca7eb481db 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -15,28 +15,6 @@ namespace node { -inline IsolateData* IsolateData::Get(v8::Isolate* isolate) { - return static_cast(isolate->GetData(kIsolateSlot)); -} - -inline IsolateData* IsolateData::GetOrCreate(v8::Isolate* isolate, - uv_loop_t* loop) { - IsolateData* isolate_data = Get(isolate); - if (isolate_data == nullptr) { - isolate_data = new IsolateData(isolate, loop); - isolate->SetData(kIsolateSlot, isolate_data); - } - isolate_data->ref_count_ += 1; - return isolate_data; -} - -inline void IsolateData::Put() { - if (--ref_count_ == 0) { - isolate()->SetData(kIsolateSlot, nullptr); - delete this; - } -} - // Create string properties as internalized one byte strings. // // Internalized because it makes property lookups a little faster and because @@ -46,9 +24,8 @@ inline void IsolateData::Put() { // // One byte because our strings are ASCII and we can safely skip V8's UTF-8 // decoding step. It's a one-time cost, but why pay it when you don't have to? -inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* loop) - : event_loop_(loop), - isolate_(isolate), +inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop) + : #define V(PropertyName, StringValue) \ PropertyName ## _( \ isolate, \ @@ -71,16 +48,12 @@ inline IsolateData::IsolateData(v8::Isolate* isolate, uv_loop_t* loop) sizeof(StringValue) - 1).ToLocalChecked()), PER_ISOLATE_STRING_PROPERTIES(V) #undef V - ref_count_(0) {} + isolate_(isolate), event_loop_(event_loop) {} inline uv_loop_t* IsolateData::event_loop() const { return event_loop_; } -inline v8::Isolate* IsolateData::isolate() const { - return isolate_; -} - inline Environment::AsyncHooks::AsyncHooks() { for (int i = 0; i < kFieldsCount; i++) fields_[i] = 0; } @@ -176,9 +149,9 @@ inline void Environment::ArrayBufferAllocatorInfo::reset_fill_flag() { fields_[kNoZeroFill] = 0; } -inline Environment* Environment::New(v8::Local context, - uv_loop_t* loop) { - Environment* env = new Environment(context, loop); +inline Environment* Environment::New(IsolateData* isolate_data, + v8::Local context) { + Environment* env = new Environment(isolate_data, context); env->AssignToContext(context); return env; } @@ -212,11 +185,11 @@ inline Environment* Environment::GetCurrent( return static_cast(data.As()->Value()); } -inline Environment::Environment(v8::Local context, - uv_loop_t* loop) +inline Environment::Environment(IsolateData* isolate_data, + v8::Local context) : isolate_(context->GetIsolate()), - isolate_data_(IsolateData::GetOrCreate(context->GetIsolate(), loop)), - timer_base_(uv_now(loop)), + isolate_data_(isolate_data), + timer_base_(uv_now(isolate_data->event_loop())), using_domains_(false), printed_error_(false), trace_sync_io_(false), @@ -253,7 +226,6 @@ inline Environment::~Environment() { #define V(PropertyName, TypeName) PropertyName ## _.Reset(); ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V) #undef V - isolate_data()->Put(); delete[] heap_statistics_buffer_; delete[] heap_space_statistics_buffer_; @@ -541,9 +513,9 @@ inline v8::Local Environment::NewInternalFieldObject() { #define VS(PropertyName, StringValue) V(v8::String, PropertyName, StringValue) #define V(TypeName, PropertyName, StringValue) \ inline \ - v8::Local IsolateData::PropertyName() const { \ + v8::Local IsolateData::PropertyName(v8::Isolate* isolate) const { \ /* Strings are immutable so casting away const-ness here is okay. */ \ - return const_cast(this)->PropertyName ## _.Get(isolate()); \ + return const_cast(this)->PropertyName ## _.Get(isolate); \ } PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_STRING_PROPERTIES(VS) @@ -555,7 +527,7 @@ inline v8::Local Environment::NewInternalFieldObject() { #define VS(PropertyName, StringValue) V(v8::String, PropertyName, StringValue) #define V(TypeName, PropertyName, StringValue) \ inline v8::Local Environment::PropertyName() const { \ - return isolate_data()->PropertyName(); \ + return isolate_data()->PropertyName(isolate()); \ } PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_STRING_PROPERTIES(VS) diff --git a/src/env.h b/src/env.h index bbbd3f4b36e68d..a6aea815fcd75a 100644 --- a/src/env.h +++ b/src/env.h @@ -304,14 +304,13 @@ RB_HEAD(ares_task_list, ares_task_t); class IsolateData { public: - static inline IsolateData* GetOrCreate(v8::Isolate* isolate, uv_loop_t* loop); - inline void Put(); + inline IsolateData(v8::Isolate* isolate, uv_loop_t* event_loop); inline uv_loop_t* event_loop() const; #define VP(PropertyName, StringValue) V(v8::Private, PropertyName, StringValue) #define VS(PropertyName, StringValue) V(v8::String, PropertyName, StringValue) #define V(TypeName, PropertyName, StringValue) \ - inline v8::Local PropertyName() const; + inline v8::Local PropertyName(v8::Isolate* isolate) const; PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP) PER_ISOLATE_STRING_PROPERTIES(VS) #undef V @@ -319,15 +318,6 @@ class IsolateData { #undef VP private: - static const int kIsolateSlot = NODE_ISOLATE_SLOT; - - inline static IsolateData* Get(v8::Isolate* isolate); - inline explicit IsolateData(v8::Isolate* isolate, uv_loop_t* loop); - inline v8::Isolate* isolate() const; - - uv_loop_t* const event_loop_; - v8::Isolate* const isolate_; - #define VP(PropertyName, StringValue) V(v8::Private, PropertyName, StringValue) #define VS(PropertyName, StringValue) V(v8::String, PropertyName, StringValue) #define V(TypeName, PropertyName, StringValue) \ @@ -338,7 +328,8 @@ class IsolateData { #undef VS #undef VP - unsigned int ref_count_; + v8::Isolate* const isolate_; + uv_loop_t* const event_loop_; DISALLOW_COPY_AND_ASSIGN(IsolateData); }; @@ -474,8 +465,8 @@ class Environment { const v8::PropertyCallbackInfo& info); // See CreateEnvironment() in src/node.cc. - static inline Environment* New(v8::Local context, - uv_loop_t* loop); + static inline Environment* New(IsolateData* isolate_data, + v8::Local context); inline void CleanupHandles(); inline void Dispose(); @@ -609,7 +600,7 @@ class Environment { static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX; private: - inline Environment(v8::Local context, uv_loop_t* loop); + inline Environment(IsolateData* isolate_data, v8::Local context); inline ~Environment(); inline IsolateData* isolate_data() const; diff --git a/src/node.cc b/src/node.cc index 258ebb596a08ca..025c1a921a884b 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4264,42 +4264,6 @@ int EmitExit(Environment* env) { } -// Just a convenience method -Environment* CreateEnvironment(Isolate* isolate, - Local context, - int argc, - const char* const* argv, - int exec_argc, - const char* const* exec_argv) { - Environment* env; - Context::Scope context_scope(context); - - env = CreateEnvironment(isolate, - uv_default_loop(), - context, - argc, - argv, - exec_argc, - exec_argv); - - LoadEnvironment(env); - - return env; -} - -static Environment* CreateEnvironment(Isolate* isolate, - Local context, - NodeInstanceData* instance_data) { - return CreateEnvironment(isolate, - instance_data->event_loop(), - context, - instance_data->argc(), - instance_data->argv(), - instance_data->exec_argc(), - instance_data->exec_argv()); -} - - static void HandleCloseCb(uv_handle_t* handle) { Environment* env = reinterpret_cast(handle->data); env->FinishHandleCleanup(handle); @@ -4314,17 +4278,27 @@ static void HandleCleanup(Environment* env, } -Environment* CreateEnvironment(Isolate* isolate, - uv_loop_t* loop, +IsolateData* CreateIsolateData(Isolate* isolate, uv_loop_t* loop) { + return new IsolateData(isolate, loop); +} + + +void FreeIsolateData(IsolateData* isolate_data) { + delete isolate_data; +} + + +Environment* CreateEnvironment(IsolateData* isolate_data, Local context, int argc, const char* const* argv, int exec_argc, const char* const* exec_argv) { + Isolate* isolate = context->GetIsolate(); HandleScope handle_scope(isolate); Context::Scope context_scope(context); - Environment* env = Environment::New(context, loop); + Environment* env = Environment::New(isolate_data, context); isolate->SetAutorunMicrotasks(false); @@ -4412,10 +4386,23 @@ static void StartNodeInstance(void* arg) { Isolate::Scope isolate_scope(isolate); HandleScope handle_scope(isolate); Local context = Context::New(isolate); - Environment* env = CreateEnvironment(isolate, context, instance_data); - array_buffer_allocator->set_env(env); Context::Scope context_scope(context); + // FIXME(bnoordhuis) Work around V8 bug: v8::Private::ForApi() dereferences + // a nullptr when a context hasn't been entered first. The symbol registry + // is a lazily created JS object but object instantiation does not work + // without a context. + IsolateData isolate_data(isolate, instance_data->event_loop()); + + Environment* env = CreateEnvironment(&isolate_data, + context, + instance_data->argc(), + instance_data->argv(), + instance_data->exec_argc(), + instance_data->exec_argv()); + + array_buffer_allocator->set_env(env); + isolate->SetAbortOnUncaughtExceptionCallback( ShouldAbortOnUncaughtException); diff --git a/src/node.h b/src/node.h index 42c5ac59d7ecf2..c1c149cdc30eb9 100644 --- a/src/node.h +++ b/src/node.h @@ -190,28 +190,22 @@ NODE_EXTERN void Init(int* argc, int* exec_argc, const char*** exec_argv); +class IsolateData; class Environment; -NODE_EXTERN Environment* CreateEnvironment(v8::Isolate* isolate, - struct uv_loop_s* loop, - v8::Local context, - int argc, - const char* const* argv, - int exec_argc, - const char* const* exec_argv); -NODE_EXTERN void LoadEnvironment(Environment* env); -NODE_EXTERN void FreeEnvironment(Environment* env); +NODE_EXTERN IsolateData* CreateIsolateData(v8::Isolate* isolate, + struct uv_loop_s* loop); +NODE_EXTERN void FreeIsolateData(IsolateData* isolate_data); -// NOTE: Calling this is the same as calling -// CreateEnvironment() + LoadEnvironment() from above. -// `uv_default_loop()` will be passed as `loop`. -NODE_EXTERN Environment* CreateEnvironment(v8::Isolate* isolate, +NODE_EXTERN Environment* CreateEnvironment(IsolateData* isolate_data, v8::Local context, int argc, const char* const* argv, int exec_argc, const char* const* exec_argv); +NODE_EXTERN void LoadEnvironment(Environment* env); +NODE_EXTERN void FreeEnvironment(Environment* env); NODE_EXTERN void EmitBeforeExit(Environment* env); NODE_EXTERN int EmitExit(Environment* env);