mirrors/electron
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 1 Wiki Activity Actions 1

fix: maintain a ref count for objects sent over remote (#17464)

Browse source 
* spec: clean up after a failed window count assertion

Previously when this assertion failed all tests that ran after the
failed assertion also failed.  This ensure that the assertion fails for
the test that actually caused the issue but cleans up the left-over
windows so that future tests do not fail.

* fix: maintain a ref count for objects sent over remote

Previously there was a race condition where a GC could occur in the
renderer process between the main process sending a meta.id and the
renderer pulling the proxy out its weakmap to stop it being GC'ed.

This fixes that race condition by maintaining a "sent" ref count in the
object registry and a "received" ref count in the object cache on the
renderer side.  The deref request now sends the number of refs the
renderer thinks it owns, if the number does not match the value in the
object registry it is assumed that there is an IPC message containing a
new reference in flight and this race condition was hit.

The browser side ref count is then reduced and we wait for the new deref
message.  This guaruntees that an object will only be removed from the
registry if every reference we sent has been guarunteed to be unreffed.
This commit is contained in:
Samuel Attard 2019-04-16 16:08:11 -04:00 • committed by GitHub
parent 9c3315c416
commit 78411db4b5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 75 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

1
atom/common/api/atom_api_v8_util.cc
View file

@ -115,6 +115,7 @@ void Initialize(v8::Local<v8::Object> exports,
dict.SetMethod("takeHeapSnapshot", &TakeHeapSnapshot);
dict.SetMethod("setRemoteCallbackFreer", &atom::RemoteCallbackFreer::BindTo);
dict.SetMethod("setRemoteObjectFreer", &atom::RemoteObjectFreer::BindTo);
dict.SetMethod("addRemoteObjectRef", &atom::RemoteObjectFreer::AddRef);
dict.SetMethod("createIDWeakMap", &atom::api::KeyWeakMap<int32_t>::Create);
dict.SetMethod(
"createDoubleIDWeakMap",

14
atom/common/api/remote_object_freer.cc
View file

@ -36,6 +36,14 @@ void RemoteObjectFreer::BindTo(v8::Isolate* isolate,
new RemoteObjectFreer(isolate, target, context_id, object_id);
}
// static
void RemoteObjectFreer::AddRef(const std::string& context_id, int object_id) {
ref_mapper_[context_id][object_id]++;
}
// static
std::map<std::string, std::map<int, int>> RemoteObjectFreer::ref_mapper_;
RemoteObjectFreer::RemoteObjectFreer(v8::Isolate* isolate,
v8::Local<v8::Object> target,
const std::string& context_id,
@ -62,6 +70,12 @@ void RemoteObjectFreer::RunDestructor() {
base::ListValue args;
args.AppendString(context_id_);
args.AppendInteger(object_id_);
args.AppendInteger(ref_mapper_[context_id_][object_id_]);
// Reset our local ref count in case we are in a GC race condition and will
// get more references in an inbound IPC message
ref_mapper_[context_id_].erase(object_id_);
if (ref_mapper_[context_id_].empty())
ref_mapper_.erase(context_id_);
mojom::ElectronBrowserAssociatedPtr electron_ptr;
render_frame->GetRemoteAssociatedInterfaces()->GetInterface(

5
atom/common/api/remote_object_freer.h
View file

@ -5,6 +5,7 @@
#ifndef ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
#define ATOM_COMMON_API_REMOTE_OBJECT_FREER_H_
#include <map>
#include <string>
#include "atom/common/api/object_life_monitor.h"
@ -17,6 +18,7 @@ class RemoteObjectFreer : public ObjectLifeMonitor {
v8::Local<v8::Object> target,
const std::string& context_id,
int object_id);
static void AddRef(const std::string& context_id, int object_id);
protected:
RemoteObjectFreer(v8::Isolate* isolate,
@ -27,6 +29,9 @@ class RemoteObjectFreer : public ObjectLifeMonitor {
void RunDestructor() override;
// { context_id => { object_id => ref_count }}
static std::map<std::string, std::map<int, int>> ref_mapper_;
private:
std::string context_id_;
int object_id_;