Only deregister Managed destructors on second pass callback

This will ensure that the Isolate tear-down sequence runs the
destructors even if the first callback already ran. The second callback
might never be executed, since it runs on a background thread which
might not be executed before tear-down of the Isolate.

R=titzer@chromium.org, mlippautz@chromium.org

Bug: v8:8208, chromium:893549
Change-Id: Ibc148e71bc06936033c5b0f0e70bbb2b5a8e8094
Reviewed-on: https://chromium-review.googlesource.com/c/1276629Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56592}

src/isolate.cc

@@ -2417,6 +2417,7 @@ char* Isolate::RestoreThread(char* from) {
 }
 
 void Isolate::ReleaseSharedPtrs() {
+  base::LockGuard<base::Mutex> lock(&managed_ptr_destructors_mutex_);
   while (managed_ptr_destructors_head_) {
     ManagedPtrDestructor* l = managed_ptr_destructors_head_;
     ManagedPtrDestructor* n = nullptr;
@@ -2430,6 +2431,7 @@ void Isolate::ReleaseSharedPtrs() {
 }
 
 void Isolate::RegisterManagedPtrDestructor(ManagedPtrDestructor* destructor) {
+  base::LockGuard<base::Mutex> lock(&managed_ptr_destructors_mutex_);
   DCHECK_NULL(destructor->prev_);
   DCHECK_NULL(destructor->next_);
   if (managed_ptr_destructors_head_) {
@@ -2440,6 +2442,7 @@ void Isolate::RegisterManagedPtrDestructor(ManagedPtrDestructor* destructor) {
 }
 
 void Isolate::UnregisterManagedPtrDestructor(ManagedPtrDestructor* destructor) {
+  base::LockGuard<base::Mutex> lock(&managed_ptr_destructors_mutex_);
   if (destructor->prev_) {
     destructor->prev_->next_ = destructor->next_;
   } else {

src/isolate.h

@@ -1908,6 +1908,7 @@ class Isolate : private HiddenFactory {
 
   bool allow_atomics_wait_;
 
+  base::Mutex managed_ptr_destructors_mutex_;
   ManagedPtrDestructor* managed_ptr_destructors_head_ = nullptr;
 
   size_t total_regexp_code_generated_;

src/objects/managed.cc

@@ -13,6 +13,8 @@ namespace {
 void ManagedObjectFinalizerSecondPass(const v8::WeakCallbackInfo<void>& data) {
   auto destructor =
       reinterpret_cast<ManagedPtrDestructor*>(data.GetParameter());
+  Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
+  isolate->UnregisterManagedPtrDestructor(destructor);
   int64_t adjustment = 0 - static_cast<int64_t>(destructor->estimated_size_);
   destructor->destructor_(destructor->shared_ptr_ptr_);
   delete destructor;
@@ -26,8 +28,6 @@ void ManagedObjectFinalizer(const v8::WeakCallbackInfo<void>& data) {
   auto destructor =
       reinterpret_cast<ManagedPtrDestructor*>(data.GetParameter());
   GlobalHandles::Destroy(destructor->global_handle_location_);
-  Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
-  isolate->UnregisterManagedPtrDestructor(destructor);
   // We need to do the main work as a second pass callback because
   // it can trigger garbage collection. The first pass callbacks
   // are not allowed to invoke V8 API.