From eae7c840b7ce8a954c67efb60553102dcde9f106 Mon Sep 17 00:00:00 2001 From: Robo Date: Wed, 28 Oct 2015 21:29:46 +0530 Subject: [PATCH] use idweakmap for holding callbacks in browser --- atom/browser/lib/rpc-server.coffee | 13 ++++---- atom/common/api/atom_api_v8_util.cc | 8 +++++ atom/common/api/lib/callbacks-registry.coffee | 12 ++++---- atom/common/id_weak_map.cc | 30 +++++++++++++++++++ atom/common/id_weak_map.h | 11 ++++++- atom/renderer/api/lib/remote.coffee | 2 +- 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/atom/browser/lib/rpc-server.coffee b/atom/browser/lib/rpc-server.coffee index c4d91830b5..13c8ceb16e 100644 --- a/atom/browser/lib/rpc-server.coffee +++ b/atom/browser/lib/rpc-server.coffee @@ -3,8 +3,8 @@ path = require 'path' objectsRegistry = require './objects-registry.js' v8Util = process.atomBinding 'v8_util' -# caches callback with their registry ID. -rendererCallbacks = {} +# weak refereence to callback with their registry ID. +rendererCallbacks = v8Util.createWeakMap() # Convert a real value into meta data. valueToMeta = (sender, value, optimizeSimpleObject=false) -> @@ -77,18 +77,19 @@ unwrapArgs = (sender, args) -> objectsRegistry.once "clear-#{sender.getId()}", -> rendererReleased = true - return rendererCallbacks[meta.id] if rendererCallbacks[meta.id]? + if rendererCallbacks.has(meta.id) + return rendererCallbacks.get(meta.id) ret = -> if rendererReleased throw new Error("Attempting to call a function in a renderer window - that has been closed or released. Function provided here: #{meta.id}.") + that has been closed or released. Function provided here: #{meta.location}.") sender.send 'ATOM_RENDERER_CALLBACK', meta.id, valueToMeta(sender, arguments) v8Util.setDestructor ret, -> return if rendererReleased - delete rendererCallbacks[meta.id] + rendererCallbacks.remove meta.id sender.send 'ATOM_RENDERER_RELEASE_CALLBACK', meta.id - rendererCallbacks[meta.id] = ret + rendererCallbacks.set meta.id, ret ret else throw new TypeError("Unknown type: #{meta.type}") diff --git a/atom/common/api/atom_api_v8_util.cc b/atom/common/api/atom_api_v8_util.cc index bba3399a8d..4a321f84d8 100644 --- a/atom/common/api/atom_api_v8_util.cc +++ b/atom/common/api/atom_api_v8_util.cc @@ -3,7 +3,9 @@ // found in the LICENSE file. #include "atom/common/api/object_life_monitor.h" +#include "atom/common/id_weak_map.h" #include "atom/common/node_includes.h" +#include "native_mate/handle.h" #include "native_mate/dictionary.h" #include "v8/include/v8-profiler.h" @@ -46,6 +48,11 @@ void TakeHeapSnapshot(v8::Isolate* isolate) { isolate->GetHeapProfiler()->TakeHeapSnapshot(); } +mate::Handle CreateWeakMap(v8::Isolate* isolate) { + auto handle = mate::CreateHandle(isolate, new atom::IDWeakMap); + return handle; +} + void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); @@ -56,6 +63,7 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("getObjectHash", &GetObjectHash); dict.SetMethod("setDestructor", &SetDestructor); dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot); + dict.SetMethod("createWeakMap", &CreateWeakMap); } } // namespace diff --git a/atom/common/api/lib/callbacks-registry.coffee b/atom/common/api/lib/callbacks-registry.coffee index 5151640b2b..b777f7a462 100644 --- a/atom/common/api/lib/callbacks-registry.coffee +++ b/atom/common/api/lib/callbacks-registry.coffee @@ -1,3 +1,5 @@ +v8Util = process.atomBinding 'v8_util' + module.exports = class CallbacksRegistry constructor: -> @@ -7,10 +9,6 @@ class CallbacksRegistry add: (callback) -> id = ++@nextId - for id,value of @callbacks - if value == callback - return id - # Capture the location of the function and put it in the ID string, # so that release errors can be tracked down easily. regexp = /at (.*)/gi @@ -21,10 +19,14 @@ class CallbacksRegistry continue if location.indexOf('(native)') isnt -1 continue if location.indexOf('atom.asar') isnt -1 [x, filenameAndLine] = /([^/^\)]*)\)?$/gi.exec(location) - id = "#{filenameAndLine} (#{id})" break + if v8Util.getHiddenValue(callback, 'metaId')? + return v8Util.getHiddenValue(callback, 'metaId') + @callbacks[id] = callback + v8Util.setHiddenValue callback, 'metaId', id + v8Util.setHiddenValue callback, 'location', filenameAndLine id get: (id) -> diff --git a/atom/common/id_weak_map.cc b/atom/common/id_weak_map.cc index c5c4b60cac..3d7e151359 100644 --- a/atom/common/id_weak_map.cc +++ b/atom/common/id_weak_map.cc @@ -8,6 +8,18 @@ #include "native_mate/converter.h" +namespace mate { + +template +struct Converter> { + static v8::Local ToV8(v8::Isolate* isolate, + v8::MaybeLocal val) { + return ConvertToV8(isolate, val.ToLocalChecked()); + } +}; + +} // namespace mate + namespace atom { namespace { @@ -41,6 +53,15 @@ int32_t IDWeakMap::Add(v8::Isolate* isolate, v8::Local object) { return id; } +void IDWeakMap::Set(v8::Isolate* isolate, + int32_t id, + v8::Local object) { + auto global = make_linked_ptr(new v8::Global(isolate, object)); + ObjectKey* key = new ObjectKey(id, this); + global->SetWeak(key, OnObjectGC, v8::WeakCallbackType::kParameter); + map_[id] = global; +} + v8::MaybeLocal IDWeakMap::Get(v8::Isolate* isolate, int32_t id) { auto iter = map_.find(id); if (iter == map_.end()) @@ -85,4 +106,13 @@ int32_t IDWeakMap::GetNextID() { return ++next_id_; } +mate::ObjectTemplateBuilder IDWeakMap::GetObjectTemplateBuilder( + v8::Isolate* isolate) { + return mate::ObjectTemplateBuilder(isolate) + .SetMethod("set", &IDWeakMap::Set) + .SetMethod("get", &IDWeakMap::Get) + .SetMethod("has", &IDWeakMap::Has) + .SetMethod("remove", &IDWeakMap::Remove); +} + } // namespace atom diff --git a/atom/common/id_weak_map.h b/atom/common/id_weak_map.h index 9fe71ebb61..c291f76a43 100644 --- a/atom/common/id_weak_map.h +++ b/atom/common/id_weak_map.h @@ -9,12 +9,14 @@ #include #include "base/memory/linked_ptr.h" +#include "native_mate/object_template_builder.h" +#include "native_mate/wrappable.h" #include "v8/include/v8.h" namespace atom { // Like ES6's WeakMap, but the key is Integer and the value is Weak Pointer. -class IDWeakMap { +class IDWeakMap : public mate::Wrappable { public: IDWeakMap(); ~IDWeakMap(); @@ -22,6 +24,9 @@ class IDWeakMap { // Adds |object| to WeakMap and returns its allocated |id|. int32_t Add(v8::Isolate* isolate, v8::Local object); + // Sets the object to WeakMap with the given |id|. + void Set(v8::Isolate* isolate, int32_t id, v8::Local object); + // Gets the object from WeakMap by its |id|. v8::MaybeLocal Get(v8::Isolate* isolate, int32_t id); @@ -40,6 +45,10 @@ class IDWeakMap { // Clears the weak map. void Clear(); + protected: + mate::ObjectTemplateBuilder GetObjectTemplateBuilder( + v8::Isolate* isolate) override; + private: // Returns next available ID. int32_t GetNextID(); diff --git a/atom/renderer/api/lib/remote.coffee b/atom/renderer/api/lib/remote.coffee index 8a5565f065..b5a3a694ee 100644 --- a/atom/renderer/api/lib/remote.coffee +++ b/atom/renderer/api/lib/remote.coffee @@ -33,7 +33,7 @@ wrapArgs = (args, visited=[]) -> else if typeof value is 'function' and v8Util.getHiddenValue value, 'returnValue' type: 'function-with-return-value', value: valueToMeta(value()) else if typeof value is 'function' - type: 'function', id: callbacksRegistry.add(value) + type: 'function', id: callbacksRegistry.add(value), location: v8Util.getHiddenValue value, 'location' else type: 'value', value: value