Skip to content

Commit

Permalink
src: use stack-allocated Environment instances
Browse files Browse the repository at this point in the history 
Makes it easier to reason about the lifetime of the Environment object.

PR-URL: #7090
Refs: #7082
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
@bnoordhuis
bnoordhuis committed Jun 2, 2016
1 parent 58cec4e commit aac79df
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 80 deletions.
27 changes: 10 additions & 17 deletions src/debug-agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,31 +174,24 @@ void Agent::WorkerRun() {
Local<Context> context = Context::New(isolate);

Context::Scope context_scope(context);
Environment* env = CreateEnvironment(
&isolate_data,
context,
arraysize(argv),
argv,
arraysize(argv),
argv);
Environment env(&isolate_data, context);

child_env_ = env;
const bool start_profiler_idle_notifier = false;
env.Start(arraysize(argv), argv,
arraysize(argv), argv,
start_profiler_idle_notifier);

child_env_ = &env;

// Expose API
InitAdaptor(env);
LoadEnvironment(env);
InitAdaptor(&env);
LoadEnvironment(&env);

CHECK_EQ(&child_loop_, env->event_loop());
CHECK_EQ(&child_loop_, env.event_loop());
uv_run(&child_loop_, UV_RUN_DEFAULT);

// Clean-up peristent
api_.Reset();

// Clean-up all running handles
env->CleanupHandles();

env->Dispose();
env = nullptr;
}
isolate->Dispose();
}
Expand Down
34 changes: 11 additions & 23 deletions src/env-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,6 @@ inline void Environment::TickInfo::set_index(uint32_t value) {
fields_[kIndex] = value;
}

inline Environment* Environment::New(IsolateData* isolate_data,
v8::Local<v8::Context> context) {
Environment* env = new Environment(isolate_data, context);
env->AssignToContext(context);
return env;
}

inline void Environment::AssignToContext(v8::Local<v8::Context> context) {
context->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex, this);
}
Expand Down Expand Up @@ -184,6 +177,7 @@ inline Environment::Environment(IsolateData* isolate_data,
#if HAVE_INSPECTOR
inspector_agent_(this),
#endif
handle_cleanup_waiting_(0),
http_parser_buffer_(nullptr),
context_(context->GetIsolate(), context) {
// We'll be creating new objects so make sure we've entered the context.
Expand All @@ -200,24 +194,12 @@ inline Environment::Environment(IsolateData* isolate_data,
set_generic_internal_field_template(obj);

RB_INIT(&cares_task_list_);
handle_cleanup_waiting_ = 0;
AssignToContext(context);
}

inline Environment::~Environment() {
v8::HandleScope handle_scope(isolate());

context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
nullptr);
#define V(PropertyName, TypeName) PropertyName ## _.Reset();
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
#undef V

delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
delete[] http_parser_buffer_;
}

inline void Environment::CleanupHandles() {
while (HandleCleanup* hc = handle_cleanup_queue_.PopFront()) {
handle_cleanup_waiting_++;
hc->cb_(this, hc->handle_, hc->arg_);
Expand All @@ -226,10 +208,16 @@ inline void Environment::CleanupHandles() {

while (handle_cleanup_waiting_ != 0)
uv_run(event_loop(), UV_RUN_ONCE);
}

inline void Environment::Dispose() {
delete this;
context()->SetAlignedPointerInEmbedderData(kContextEmbedderDataIndex,
nullptr);
#define V(PropertyName, TypeName) PropertyName ## _.Reset();
ENVIRONMENT_STRONG_PERSISTENT_PROPERTIES(V)
#undef V

delete[] heap_statistics_buffer_;
delete[] heap_space_statistics_buffer_;
delete[] http_parser_buffer_;
}

inline v8::Isolate* Environment::isolate() const {
Expand Down
14 changes: 4 additions & 10 deletions src/env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,17 +446,14 @@ class Environment {
static inline Environment* GetCurrent(
const v8::PropertyCallbackInfo<T>& info);

// See CreateEnvironment() in src/node.cc.
static inline Environment* New(IsolateData* isolate_data,
v8::Local<v8::Context> context);
inline Environment(IsolateData* isolate_data, v8::Local<v8::Context> context);
inline ~Environment();

void Start(int argc,
const char* const* argv,
int exec_argc,
const char* const* exec_argv,
bool start_profiler_idle_notifier);
inline void CleanupHandles();
inline void Dispose();

void AssignToContext(v8::Local<v8::Context> context);

void StartProfilerIdleNotifier();
Expand All @@ -472,7 +469,7 @@ class Environment {
inline uv_check_t* immediate_check_handle();
inline uv_idle_t* immediate_idle_handle();

// Register clean-up cb to be called on env->Dispose()
// Register clean-up cb to be called on environment destruction.
inline void RegisterHandleCleanup(uv_handle_t* handle,
HandleCleanupCb cb,
void *arg);
Expand Down Expand Up @@ -584,9 +581,6 @@ class Environment {
static const int kContextEmbedderDataIndex = NODE_CONTEXT_EMBEDDER_DATA_INDEX;

private:
inline Environment(IsolateData* isolate_data, v8::Local<v8::Context> context);
inline ~Environment();

v8::Isolate* const isolate_;
IsolateData* const isolate_data_;
uv_check_t immediate_check_handle_;
Expand Down
56 changes: 26 additions & 30 deletions src/node.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3363,12 +3363,6 @@ void LoadEnvironment(Environment* env) {
}


void FreeEnvironment(Environment* env) {
CHECK_NE(env, nullptr);
env->Dispose();
}


static void PrintHelp();

static bool ParseDebugOpt(const char* arg) {
Expand Down Expand Up @@ -4256,12 +4250,17 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
Isolate* isolate = context->GetIsolate();
HandleScope handle_scope(isolate);
Context::Scope context_scope(context);
Environment* env = Environment::New(isolate_data, context);
auto env = new Environment(isolate_data, context);
env->Start(argc, argv, exec_argc, exec_argv, v8_is_profiling);
return env;
}


void FreeEnvironment(Environment* env) {
delete env;
}


// Entry point for new node instances, also called directly for the main
// node instance.
static void StartNodeInstance(void* arg) {
Expand Down Expand Up @@ -4293,60 +4292,60 @@ static void StartNodeInstance(void* arg) {
array_buffer_allocator.zero_fill_field());
Local<Context> context = Context::New(isolate);
Context::Scope context_scope(context);
Environment* env = CreateEnvironment(&isolate_data,
context,
instance_data->argc(),
instance_data->argv(),
instance_data->exec_argc(),
instance_data->exec_argv());
Environment env(&isolate_data, context);
env.Start(instance_data->argc(),
instance_data->argv(),
instance_data->exec_argc(),
instance_data->exec_argv(),
v8_is_profiling);

isolate->SetAbortOnUncaughtExceptionCallback(
ShouldAbortOnUncaughtException);

// Start debug agent when argv has --debug
if (instance_data->use_debug_agent())
StartDebug(env, debug_wait_connect);
StartDebug(&env, debug_wait_connect);

{
Environment::AsyncCallbackScope callback_scope(env);
LoadEnvironment(env);
Environment::AsyncCallbackScope callback_scope(&env);
LoadEnvironment(&env);
}

env->set_trace_sync_io(trace_sync_io);
env.set_trace_sync_io(trace_sync_io);

// Enable debugger
if (instance_data->use_debug_agent())
EnableDebug(env);
EnableDebug(&env);

{
SealHandleScope seal(isolate);
bool more;
do {
v8::platform::PumpMessageLoop(default_platform, isolate);
more = uv_run(env->event_loop(), UV_RUN_ONCE);
more = uv_run(env.event_loop(), UV_RUN_ONCE);

if (more == false) {
v8::platform::PumpMessageLoop(default_platform, isolate);
EmitBeforeExit(env);
EmitBeforeExit(&env);

// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.
more = uv_loop_alive(env->event_loop());
if (uv_run(env->event_loop(), UV_RUN_NOWAIT) != 0)
more = uv_loop_alive(env.event_loop());
if (uv_run(env.event_loop(), UV_RUN_NOWAIT) != 0)
more = true;
}
} while (more == true);
}

env->set_trace_sync_io(false);
env.set_trace_sync_io(false);

int exit_code = EmitExit(env);
int exit_code = EmitExit(&env);
if (instance_data->is_main())
instance_data->set_exit_code(exit_code);
RunAtExit(env);
RunAtExit(&env);

#if HAVE_INSPECTOR
if (env->inspector_agent()->connected()) {
if (env.inspector_agent()->connected()) {
// Restore signal dispositions, the app is done and is no longer
// capable of handling signals.
#ifdef __POSIX__
Expand All @@ -4359,16 +4358,13 @@ static void StartNodeInstance(void* arg) {
CHECK_EQ(0, sigaction(nr, &act, nullptr));
}
#endif
env->inspector_agent()->WaitForDisconnect();
env.inspector_agent()->WaitForDisconnect();
}
#endif

#if defined(LEAK_SANITIZER)
__lsan_do_leak_check();
#endif

env->Dispose();
env = nullptr;
}

uv_mutex_lock(&node_isolate_mutex);
Expand Down

0 comments on commit aac79df

Please sign in to comment.