Browse Source

fix: avoid double-free in TrackableObject (#22768)

Cheng Zhao 5 years ago
parent
commit
a9b9016b99

+ 1 - 0
shell/common/gin_helper/trackable_object.h

@@ -58,6 +58,7 @@ class TrackableObject : public TrackableObjectBase, public EventEmitter<T> {
     v8::Local<v8::Object> wrapper = gin_helper::Wrappable<T>::GetWrapper();
     if (!wrapper.IsEmpty()) {
       wrapper->SetAlignedPointerInInternalField(0, nullptr);
+      gin_helper::WrappableBase::wrapper_.ClearWeak();
     }
   }
 

+ 12 - 14
shell/common/gin_helper/wrappable.cc

@@ -49,7 +49,8 @@ void WrappableBase::InitWith(v8::Isolate* isolate,
   isolate_ = isolate;
   wrapper->SetAlignedPointerInInternalField(0, this);
   wrapper_.Reset(isolate, wrapper);
-  wrapper_.SetWeak(this, FirstWeakCallback, v8::WeakCallbackType::kParameter);
+  wrapper_.SetWeak(this, FirstWeakCallback,
+                   v8::WeakCallbackType::kInternalFields);
 
   // Call object._init if we have one.
   v8::Local<v8::Function> init;
@@ -62,24 +63,21 @@ void WrappableBase::InitWith(v8::Isolate* isolate,
 // static
 void WrappableBase::FirstWeakCallback(
     const v8::WeakCallbackInfo<WrappableBase>& data) {
-  WrappableBase* wrappable = data.GetParameter();
-  wrappable->wrapper_.Reset();
-  data.SetSecondPassCallback(SecondWeakCallback);
+  WrappableBase* wrappable =
+      static_cast<WrappableBase*>(data.GetInternalField(0));
+  if (wrappable) {
+    wrappable->wrapper_.Reset();
+    data.SetSecondPassCallback(SecondWeakCallback);
+  }
 }
 
 // static
 void WrappableBase::SecondWeakCallback(
     const v8::WeakCallbackInfo<WrappableBase>& data) {
-  // Certain classes (for example api::WebContents and api::WebContentsView)
-  // are running JS code in the destructor, while V8 may crash when JS code
-  // runs inside weak callback.
-  //
-  // We work around this problem by delaying the deletion to next tick where
-  // garbage collection is done.
-  base::ThreadTaskRunnerHandle::Get()->PostNonNestableTask(
-      FROM_HERE,
-      base::BindOnce([](WrappableBase* wrappable) { delete wrappable; },
-                     base::Unretained(data.GetParameter())));
+  WrappableBase* wrappable =
+      static_cast<WrappableBase*>(data.GetInternalField(0));
+  if (wrappable)
+    delete wrappable;
 }
 
 namespace internal {

+ 2 - 1
shell/common/gin_helper/wrappable_base.h

@@ -52,6 +52,8 @@ class WrappableBase {
   // Helper to init with arguments.
   void InitWithArgs(gin::Arguments* args);
 
+  v8::Global<v8::Object> wrapper_;  // Weak
+
  private:
   static void FirstWeakCallback(
       const v8::WeakCallbackInfo<WrappableBase>& data);
@@ -59,7 +61,6 @@ class WrappableBase {
       const v8::WeakCallbackInfo<WrappableBase>& data);
 
   v8::Isolate* isolate_ = nullptr;
-  v8::Global<v8::Object> wrapper_;  // Weak
 
   DISALLOW_COPY_AND_ASSIGN(WrappableBase);
 };