env: fix use-after-free in keep-alive allocators

As per comment in the change it is possible that
`Environment:AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose`
is called from `node::FreeEnvironment(env)` after `node::Stop(env)`
was already called.

In this case platform's `per_isolate_` map won't have an entry for our
isolate and the `AddIsolateFinishedCallback` will invoke the callback
immediately leaving us with `keep_alive_allocators_` set to freed pointer.

This commit also changes the assertion in the
`AddIsolateFinishedCallback` that is triggered incorrectly.

Author: indutny

src/env.cc

@@ -1153,6 +1153,17 @@ char* Environment::Reallocate(char* data, size_t old_size, size_t size) {
 
 void Environment::AddArrayBufferAllocatorToKeepAliveUntilIsolateDispose(
     std::shared_ptr<v8::ArrayBuffer::Allocator> allocator) {
+  // It is possible that we can arrive here from `node::FreeEnvironment` when it
+  // is called after `node::Stop(env)`.
+  //
+  // In this case platform's `per_isolate_` map won't have an entry for our
+  // isolate and the `AddIsolateFinishedCallback` will invoke the callback
+  // immediately leaving us with `keep_alive_allocators_` set to freed pointer.
+  if (is_stopping() && started_cleanup_) {
+    allocator.reset();
+    return;
+  }
+
   if (keep_alive_allocators_ == nullptr) {
     MultiIsolatePlatform* platform = isolate_data()->platform();
     CHECK_NOT_NULL(platform);

src/node_platform.cc

@@ -359,10 +359,10 @@ void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
   Mutex::ScopedLock lock(per_isolate_mutex_);
   auto it = per_isolate_.find(isolate);
   if (it == per_isolate_.end()) {
-    CHECK(it->second);
     cb(data);
     return;
   }
+  CHECK(it->second);
   it->second->AddShutdownCallback(cb, data);
 }