Merge pull request #3942 from atom/remote-memory-leak

Fix memory leak in remote module

atom/common/api/atom_api_v8_util.cc

@@ -2,22 +2,51 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
+#include <map>
+#include <string>
+
 #include "atom/common/api/object_life_monitor.h"
 #include "atom/common/node_includes.h"
+#include "base/stl_util.h"
 #include "native_mate/dictionary.h"
 #include "v8/include/v8-profiler.h"
 
 namespace {
 
+// A Persistent that can be copied and will not free itself.
+template<class T>
+struct LeakedPersistentTraits {
+  typedef v8::Persistent<T, LeakedPersistentTraits<T> > LeakedPersistent;
+  static const bool kResetInDestructor = false;
+  template<class S, class M>
+  static V8_INLINE void Copy(const v8::Persistent<S, M>& source,
+                             LeakedPersistent* dest) {
+    // do nothing, just allow copy
+  }
+};
+
+// The handles are leaked on purpose.
+using FunctionTemplateHandle =
+    LeakedPersistentTraits<v8::FunctionTemplate>::LeakedPersistent;
+std::map<std::string, FunctionTemplateHandle> function_templates_;
+
 v8::Local<v8::Object> CreateObjectWithName(v8::Isolate* isolate,
-                                            v8::Local<v8::String> name) {
+                                           const std::string& name) {
+  if (name == "Object")
+    return v8::Object::New(isolate);
+
+  if (ContainsKey(function_templates_, name))
+    return v8::Local<v8::FunctionTemplate>::New(
+        isolate, function_templates_[name])->GetFunction()->NewInstance();
+
   v8::Local<v8::FunctionTemplate> t = v8::FunctionTemplate::New(isolate);
-  t->SetClassName(name);
+  t->SetClassName(mate::StringToV8(isolate, name));
+  function_templates_[name] = FunctionTemplateHandle(isolate, t);
   return t->GetFunction()->NewInstance();
 }
 
 v8::Local<v8::Value> GetHiddenValue(v8::Local<v8::Object> object,
-                                     v8::Local<v8::String> key) {
+                                    v8::Local<v8::String> key) {
   return object->GetHiddenValue(key);
 }
 

atom/renderer/api/lib/remote.coffee

@@ -65,39 +65,17 @@ metaToValue = (meta) ->
               return metaToValue obj
             else
               # Function call.
-              ret = ipcRenderer.sendSync 'ATOM_BROWSER_FUNCTION_CALL', meta.id, wrapArgs(arguments)
+              obj = ipcRenderer.sendSync 'ATOM_BROWSER_FUNCTION_CALL', meta.id, wrapArgs(arguments)
-              return metaToValue ret
+              return metaToValue obj
       else
         ret = v8Util.createObjectWithName meta.name
 
       # Polulate delegate members.
       for member in meta.members
-        do (member) ->
+        if member.type is 'function'
-          if member.type is 'function'
+          ret[member.name] = createRemoteMemberFunction meta.id, member.name
-            ret[member.name] =
+        else
-            class RemoteMemberFunction
+          Object.defineProperty ret, member.name, createRemoteMemberProperty(meta.id, member.name)
-              constructor: ->
-                if @constructor is RemoteMemberFunction
-                  # Constructor call.
-                  obj = ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', meta.id, member.name, wrapArgs(arguments)
-                  return metaToValue obj
-                else
-                  # Call member function.
-                  ret = ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_CALL', meta.id, member.name, wrapArgs(arguments)
-                  return metaToValue ret
-          else
-            Object.defineProperty ret, member.name,
-              enumerable: true,
-              configurable: false,
-              set: (value) ->
-                # Set member data.
-                ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_SET', meta.id, member.name, value
-                value
-
-              get: ->
-                # Get member data.
-                ret = ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_GET', meta.id, member.name
-                metaToValue ret
 
       # Track delegate object's life time, and tell the browser to clean up
       # when the object is GCed.
@@ -117,6 +95,35 @@ metaToPlainObject = (meta) ->
   obj[name] = value for {name, value} in meta.members
   obj
 
+# Create a RemoteMemberFunction instance.
+# This function's content should not be inlined into metaToValue, otherwise V8
+# may consider it circular reference.
+createRemoteMemberFunction = (metaId, name) ->
+  class RemoteMemberFunction
+    constructor: ->
+      if @constructor is RemoteMemberFunction
+        # Constructor call.
+        ret = ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_CONSTRUCTOR', metaId, name, wrapArgs(arguments)
+        return metaToValue ret
+      else
+        # Call member function.
+        ret = ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_CALL', metaId, name, wrapArgs(arguments)
+        return metaToValue ret
+
+# Create configuration for defineProperty.
+# This function's content should not be inlined into metaToValue, otherwise V8
+# may consider it circular reference.
+createRemoteMemberProperty = (metaId, name) ->
+  enumerable: true,
+  configurable: false,
+  set: (value) ->
+    # Set member data.
+    ipcRenderer.sendSync 'ATOM_BROWSER_MEMBER_SET', metaId, name, value
+    value
+  get: ->
+    # Get member data.
+    metaToValue ipcRenderer.sendSync('ATOM_BROWSER_MEMBER_GET', metaId, name)
+
 # Browser calls a callback in renderer.
 ipcRenderer.on 'ATOM_RENDERER_CALLBACK', (event, id, args) ->
   callbacksRegistry.apply id, metaToValue(args)