Merge pull request #8811 from electron/clean-node-on-exit

Clean up node Environment on exit

atom/browser/atom_browser_main_parts.cc

@@ -132,6 +132,7 @@ void AtomBrowserMainParts::PostEarlyInitialization() {
   // Create the global environment.
   node::Environment* env =
       node_bindings_->CreateEnvironment(js_env_->context());
+  node_env_.reset(new NodeEnvironment(env));
 
   // Make sure node can get correct environment when debugging.
   if (node_debugger_->IsRunning())

atom/browser/atom_browser_main_parts.h

@@ -22,6 +22,7 @@ class Browser;
 class JavascriptEnvironment;
 class NodeBindings;
 class NodeDebugger;
+class NodeEnvironment;
 class BridgeTaskRunner;
 
 class AtomBrowserMainParts : public brightray::BrowserMainParts {
@@ -79,6 +80,7 @@ class AtomBrowserMainParts : public brightray::BrowserMainParts {
 
   std::unique_ptr<Browser> browser_;
   std::unique_ptr<JavascriptEnvironment> js_env_;
+  std::unique_ptr<NodeEnvironment> node_env_;
   std::unique_ptr<NodeBindings> node_bindings_;
   std::unique_ptr<AtomBindings> atom_bindings_;
   std::unique_ptr<NodeDebugger> node_debugger_;

atom/browser/javascript_environment.cc

@@ -12,6 +12,8 @@
 #include "gin/array_buffer.h"
 #include "gin/v8_initializer.h"
 
+#include "atom/common/node_includes.h"
+
 namespace atom {
 
 JavascriptEnvironment::JavascriptEnvironment()
@@ -46,4 +48,11 @@ bool JavascriptEnvironment::Initialize() {
   return true;
 }
 
+NodeEnvironment::NodeEnvironment(node::Environment* env) : env_(env) {
+}
+
+NodeEnvironment::~NodeEnvironment() {
+  node::FreeEnvironment(env_);
+}
+
 }  // namespace atom

atom/browser/javascript_environment.h

@@ -8,8 +8,13 @@
 #include "base/macros.h"
 #include "gin/public/isolate_holder.h"
 
+namespace node {
+class Environment;
+}
+
 namespace atom {
 
+// Manage the V8 isolate and context automatically.
 class JavascriptEnvironment {
  public:
   JavascriptEnvironment();
@@ -37,6 +42,18 @@ class JavascriptEnvironment {
   DISALLOW_COPY_AND_ASSIGN(JavascriptEnvironment);
 };
 
+// Manage the Node Environment automatically.
+class NodeEnvironment {
+ public:
+  explicit NodeEnvironment(node::Environment* env);
+  ~NodeEnvironment();
+
+ private:
+  node::Environment* env_;
+
+  DISALLOW_COPY_AND_ASSIGN(NodeEnvironment);
+};
+
 }  // namespace atom
 
 #endif  // ATOM_BROWSER_JAVASCRIPT_ENVIRONMENT_H_

atom/common/api/atom_bindings.cc

@@ -84,6 +84,7 @@ AtomBindings::AtomBindings() {
 }
 
 AtomBindings::~AtomBindings() {
+  uv_close(reinterpret_cast<uv_handle_t*>(&call_next_tick_async_), nullptr);
 }
 
 void AtomBindings::BindTo(v8::Isolate* isolate,
@@ -117,6 +118,13 @@ void AtomBindings::BindTo(v8::Isolate* isolate,
   }
 }
 
+void AtomBindings::EnvironmentDestroyed(node::Environment* env) {
+  auto it = std::find(pending_next_ticks_.begin(), pending_next_ticks_.end(),
+                      env);
+  if (it != pending_next_ticks_.end())
+    pending_next_ticks_.erase(it);
+}
+
 void AtomBindings::ActivateUVLoop(v8::Isolate* isolate) {
   node::Environment* env = node::Environment::GetCurrent(isolate);
   if (std::find(pending_next_ticks_.begin(), pending_next_ticks_.end(), env) !=

atom/common/api/atom_bindings.h

@@ -27,6 +27,9 @@ class AtomBindings {
   // load native code from Electron instead.
   void BindTo(v8::Isolate* isolate, v8::Local<v8::Object> process);
 
+  // Should be called when a node::Environment has been destroyed.
+  void EnvironmentDestroyed(node::Environment* env);
+
   static void Log(const base::string16& message);
   static void Crash();
 

atom/common/node_bindings.cc

@@ -117,6 +117,7 @@ NodeBindings::~NodeBindings() {
 
   // Clear uv.
   uv_sem_destroy(&embed_sem_);
+  uv_close(reinterpret_cast<uv_handle_t*>(&dummy_uv_handle_), nullptr);
 }
 
 void NodeBindings::Initialize() {

atom/renderer/atom_renderer_client.cc

@@ -213,7 +213,8 @@ std::vector<std::string> ParseSchemesCLISwitch(const char* switch_name) {
 }  // namespace
 
 AtomRendererClient::AtomRendererClient()
-    : node_bindings_(NodeBindings::Create(false)),
+    : node_integration_initialized_(false),
+      node_bindings_(NodeBindings::Create(false)),
       atom_bindings_(new AtomBindings) {
   isolated_world_ = base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kContextIsolation);
@@ -349,11 +350,9 @@ void AtomRendererClient::DidCreateScriptContext(
   if (!render_frame->IsMainFrame() && !IsDevToolsExtension(render_frame))
     return;
 
-  // Whether the node binding has been initialized.
-  bool first_time = node_bindings_->uv_env() == nullptr;
-
   // Prepare the node bindings.
-  if (first_time) {
+  if (!node_integration_initialized_) {
+    node_integration_initialized_ = true;
     node_bindings_->Initialize();
     node_bindings_->PrepareMessageLoop();
   }
@@ -369,7 +368,7 @@ void AtomRendererClient::DidCreateScriptContext(
   // Load everything.
   node_bindings_->LoadEnvironment(env);
 
-  if (first_time) {
+  if (node_bindings_->uv_env() == nullptr) {
     // Make uv loop being wrapped by window context.
     node_bindings_->set_uv_env(env);
 
@@ -388,6 +387,14 @@ void AtomRendererClient::WillReleaseScriptContext(
   node::Environment* env = node::Environment::GetCurrent(context);
   if (env)
     mate::EmitEvent(env->isolate(), env->process_object(), "exit");
+
+  // The main frame may be replaced.
+  if (env == node_bindings_->uv_env())
+    node_bindings_->set_uv_env(nullptr);
+
+  // Destroy the node environment.
+  node::FreeEnvironment(env);
+  atom_bindings_->EnvironmentDestroyed(env);
 }
 
 bool AtomRendererClient::ShouldFork(blink::WebLocalFrame* frame,

atom/renderer/atom_renderer_client.h

@@ -66,6 +66,9 @@ class AtomRendererClient : public content::ContentRendererClient {
       std::vector<std::unique_ptr<::media::KeySystemProperties>>* key_systems)
       override;
 
+  // Whether the node integration has been initialized.
+  bool node_integration_initialized_;
+
   std::unique_ptr<NodeBindings> node_bindings_;
   std::unique_ptr<AtomBindings> atom_bindings_;
   std::unique_ptr<PreferencesManager> preferences_manager_;