Reland "[wasm] Lazy update instances on a shared Memory.Grow"

This is a reland of 80f06d6f

Original change's description:
> [wasm] Lazy update instances on a shared Memory.Grow
> 
>  - Introduce a GROW_SHARED_MEMORY interrupt, and handler
>  - Memory objects for isolates are updated on a stack check, add
>    tracking for isolates that hit the stack check
>  - When enough memory is not reserved ahead of time, fail to grow
>  - Add tracking for externalized buffers in the MemoryTracker so
>    that the MemoryTracker will know when backing_stores can be freed.
>  - For shared buffer, do not always allocate a new buffer when
>    growing an externalized buffer
> 
> 
> Change-Id: I9cf1be19f2f165fa6ea4096869f7d6365304c8c4
> Bug: v8:8564
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1472430
> Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
> Reviewed-by: Ben Smith <binji@chromium.org>
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#60064}

Bug: v8:8564
Change-Id: Id0cf8e42a9d54ac702dba351e248a1b92713c98a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1506357Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Deepti Gandluri <gdeepti@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60071}

src/execution.cc

@@ -640,6 +640,12 @@ Object StackGuard::HandleInterrupts() {
     isolate_->heap()->HandleGCRequest();
   }
 
+  if (CheckAndClearInterrupt(GROW_SHARED_MEMORY)) {
+    TRACE_INTERRUPT("GROW_SHARED_MEMORY");
+    isolate_->wasm_engine()->memory_tracker()->UpdateSharedMemoryInstances(
+        isolate_);
+  }
+
   if (CheckAndClearInterrupt(TERMINATE_EXECUTION)) {
     TRACE_INTERRUPT("TERMINATE_EXECUTION");
     return isolate_->TerminateExecution();

src/execution.h

@@ -91,12 +91,13 @@ class V8_EXPORT_PRIVATE StackGuard final {
   // it has been set up.
   void ClearThread(const ExecutionAccess& lock);
 
-#define INTERRUPT_LIST(V)                       \
-  V(TERMINATE_EXECUTION, TerminateExecution, 0) \
-  V(GC_REQUEST, GC, 1)                          \
-  V(INSTALL_CODE, InstallCode, 2)               \
-  V(API_INTERRUPT, ApiInterrupt, 3)             \
-  V(DEOPT_MARKED_ALLOCATION_SITES, DeoptMarkedAllocationSites, 4)
+#define INTERRUPT_LIST(V)                                         \
+  V(TERMINATE_EXECUTION, TerminateExecution, 0)                   \
+  V(GC_REQUEST, GC, 1)                                            \
+  V(INSTALL_CODE, InstallCode, 2)                                 \
+  V(API_INTERRUPT, ApiInterrupt, 3)                               \
+  V(DEOPT_MARKED_ALLOCATION_SITES, DeoptMarkedAllocationSites, 4) \
+  V(GROW_SHARED_MEMORY, GrowSharedMemory, 5)
 
 #define V(NAME, Name, id)                                                    \
   inline bool Check##Name() { return CheckInterrupt(NAME); }                 \

src/isolate.cc

@@ -2922,6 +2922,8 @@ void Isolate::Deinit() {
     optimizing_compile_dispatcher_ = nullptr;
   }
 
+  wasm_engine()->memory_tracker()->DeleteSharedMemoryObjectsOnIsolate(this);
+
   heap_.mark_compact_collector()->EnsureSweepingCompleted();
   heap_.memory_allocator()->unmapper()->EnsureUnmappingCompleted();
 

src/objects/js-array-buffer-inl.h

@@ -77,11 +77,7 @@ void* JSArrayBuffer::allocation_base() const {
 }
 
 bool JSArrayBuffer::is_wasm_memory() const {
-  bool const is_wasm_memory = IsWasmMemoryBit::decode(bit_field());
-  DCHECK_EQ(is_wasm_memory,
-            GetIsolate()->wasm_engine()->memory_tracker()->IsWasmMemory(
-                backing_store()));
-  return is_wasm_memory;
+  return IsWasmMemoryBit::decode(bit_field());
 }
 
 void JSArrayBuffer::set_is_wasm_memory(bool is_wasm_memory) {

src/objects/js-array-buffer.cc

@@ -69,11 +69,7 @@ void JSArrayBuffer::FreeBackingStore(Isolate* isolate, Allocation allocation) {
   if (allocation.is_wasm_memory) {
     wasm::WasmMemoryTracker* memory_tracker =
         isolate->wasm_engine()->memory_tracker();
-    if (!memory_tracker->FreeMemoryIfIsWasmMemory(isolate,
-                                                  allocation.backing_store)) {
-      CHECK(FreePages(GetPlatformPageAllocator(), allocation.allocation_base,
-                      allocation.length));
-    }
+    memory_tracker->FreeMemoryIfIsWasmMemory(isolate, allocation.backing_store);
   } else {
     isolate->array_buffer_allocator()->Free(allocation.allocation_base,
                                             allocation.length);

src/value-serializer.cc

@@ -922,6 +922,9 @@ Maybe<bool> ValueSerializer::WriteWasmMemory(Handle<WasmMemoryObject> object) {
     return Nothing<bool>();
   }
 
+  isolate_->wasm_engine()->memory_tracker()->RegisterWasmMemoryAsShared(
+      object, isolate_);
+
   WriteTag(SerializationTag::kWasmMemoryTransfer);
   WriteZigZag<int32_t>(object->maximum_pages());
   return WriteJSReceiver(Handle<JSReceiver>(object->array_buffer(), isolate_));
@@ -1866,6 +1869,9 @@ MaybeHandle<WasmMemoryObject> ValueDeserializer::ReadWasmMemory() {
   Handle<WasmMemoryObject> result =
       WasmMemoryObject::New(isolate_, buffer, maximum_pages);
 
+  isolate_->wasm_engine()->memory_tracker()->RegisterWasmMemoryAsShared(
+      result, isolate_);
+
   AddObjectWithID(id, result);
   return result;
 }

src/wasm/wasm-memory.h

@@ -7,6 +7,7 @@
 
 #include <atomic>
 #include <unordered_map>
+#include <unordered_set>
 
 #include "src/base/platform/mutex.h"
 #include "src/flags.h"
@@ -39,11 +40,27 @@ class WasmMemoryTracker {
                           size_t allocation_length, void* buffer_start,
                           size_t buffer_length);
 
+  struct SharedMemoryObjectState {
+    Handle<WasmMemoryObject> memory_object;
+    Isolate* isolate;
+
+    SharedMemoryObjectState() = default;
+    SharedMemoryObjectState(Handle<WasmMemoryObject> memory_object,
+                            Isolate* isolate)
+        : memory_object(memory_object), isolate(isolate) {}
+  };
+
   struct AllocationData {
     void* allocation_base = nullptr;
     size_t allocation_length = 0;
     void* buffer_start = nullptr;
     size_t buffer_length = 0;
+    bool is_shared = false;
+
+    // Track Wasm Memory instances across isolates, this is populated on
+    // PostMessage using persistent handles for memory objects.
+    std::vector<WasmMemoryTracker::SharedMemoryObjectState>
+        memory_object_vector;
 
    private:
     AllocationData() = default;
@@ -81,11 +98,11 @@ class WasmMemoryTracker {
   // Decreases the amount of reserved address space.
   void ReleaseReservation(size_t num_bytes);
 
-  // Removes an allocation from the tracker.
-  AllocationData ReleaseAllocation(Isolate* isolate, const void* buffer_start);
 
   bool IsWasmMemory(const void* buffer_start);
 
+  bool IsWasmSharedMemory(const void* buffer_start);
+
   // Returns whether the given buffer is a Wasm memory with guard regions large
   // enough to safely use trap handlers.
   bool HasFullGuardRegions(const void* buffer_start);
@@ -99,6 +116,26 @@ class WasmMemoryTracker {
   // free the buffer manually.
   bool FreeMemoryIfIsWasmMemory(Isolate* isolate, const void* buffer_start);
 
+  // When WebAssembly.Memory is transferred over PostMessage, register the
+  // allocation as shared and track the memory objects that will need
+  // updating if memory is resized.
+  void RegisterWasmMemoryAsShared(Handle<WasmMemoryObject> object,
+                                  Isolate* isolate);
+
+  // This method is called when the underlying backing store is grown, but
+  // instances that share the backing_store have not yet been updated.
+  void SetPendingUpdateOnGrow(Handle<JSArrayBuffer> old_buffer,
+                              size_t new_size);
+
+  // Interrupt handler for GROW_SHARED_MEMORY interrupt. Update memory objects
+  // and instances that share the memory objects  after a Grow call.
+  void UpdateSharedMemoryInstances(Isolate* isolate);
+
+  // Due to timing of when buffers are garbage collected, vs. when isolate
+  // object handles are destroyed, it is possible to leak global handles. To
+  // avoid this, cleanup any global handles on isolate destruction if any exist.
+  void DeleteSharedMemoryObjectsOnIsolate(Isolate* isolate);
+
   // Allocation results are reported to UMA
   //
   // See wasm_memory_allocation_result in counters.h
@@ -114,8 +151,70 @@ class WasmMemoryTracker {
   };
 
  private:
-  void AddAddressSpaceSample(Isolate* isolate);
+  // Helper methods to free memory only if not shared by other isolates, memory
+  // objects.
+  void FreeMemoryIfNotShared_Locked(Isolate* isolate,
+                                    const void* backing_store);
+  bool CanFreeSharedMemory_Locked(const void* backing_store);
+  void RemoveSharedBufferState_Locked(Isolate* isolate,
+                                      const void* backing_store);
+
+  // Registers the allocation as shared, and tracks all the memory objects
+  // associates with this allocation across isolates.
+  void RegisterSharedWasmMemory_Locked(Handle<WasmMemoryObject> object,
+                                       Isolate* isolate);
+
+  // Map the new size after grow to the buffer backing store, so that instances
+  // and memory objects that share the WebAssembly.Memory across isolates can
+  // be updated..
+  void AddBufferToGrowMap_Locked(Handle<JSArrayBuffer> old_buffer,
+                                 size_t new_size);
+
+  // Trigger a GROW_SHARED_MEMORY interrupt on all the isolates that have memory
+  // objects that share this buffer.
+  void TriggerSharedGrowInterruptOnAllIsolates_Locked(
+      Handle<JSArrayBuffer> old_buffer);
+
+  // When isolates hit a stack check, update the memory objects associated with
+  // that isolate.
+  void UpdateSharedMemoryStateOnInterrupt_Locked(Isolate* isolate,
+                                                 void* backing_store,
+                                                 size_t new_size);
+
+  // Check if all the isolates that share a backing_store have hit a stack
+  // check. If a stack check is hit, and the backing store is pending grow,
+  // this isolate will have updated memory objects.
+  bool AreAllIsolatesUpdated_Locked(const void* backing_store);
+
+  // If a grow call is made to a buffer with a pending grow, and all the
+  // isolates that share this buffer have not hit a StackCheck, clear the set of
+  // already updated instances so they can be updated with the new size on the
+  // most recent grow call.
+  void ClearUpdatedInstancesOnPendingGrow_Locked(const void* backing_store);
+
+  // Helper functions to update memory objects on grow, and maintain state for
+  // which isolates hit a stack check.
+  void UpdateMemoryObjectsForIsolate_Locked(Isolate* isolate,
+                                            void* backing_store,
+                                            size_t new_size);
+  bool MemoryObjectsNeedUpdate_Locked(Isolate* isolate,
+                                      const void* backing_store);
+
+  // Destroy global handles to memory objects, and remove backing store from
+  // isolates_per_buffer on Free.
+  void DestroyMemoryObjectsAndRemoveIsolateEntry_Locked(
+      Isolate* isolate, const void* backing_store);
+  void DestroyMemoryObjectsAndRemoveIsolateEntry_Locked(
+      const void* backing_store);
+
+  void RemoveIsolateFromBackingStore_Locked(Isolate* isolate,
+                                            const void* backing_store);
+
+  // Removes an allocation from the tracker.
+  AllocationData ReleaseAllocation_Locked(Isolate* isolate,
+                                          const void* buffer_start);
 
+  void AddAddressSpaceSample(Isolate* isolate);
   // Clients use a two-part process. First they "reserve" the address space,
   // which signifies an intent to actually allocate it. This determines whether
   // doing the allocation would put us over our limit. Once there is a
@@ -132,10 +231,36 @@ class WasmMemoryTracker {
 
   size_t allocated_address_space_ = 0;
 
+  //////////////////////////////////////////////////////////////////////////////
+  // Protected by {mutex_}:
+
   // Track Wasm memory allocation information. This is keyed by the start of the
   // buffer, rather than by the start of the allocation.
   std::unordered_map<const void*, AllocationData> allocations_;
 
+  // Maps each buffer to the isolates that share the backing store.
+  std::unordered_map<const void*, std::unordered_set<Isolate*>>
+      isolates_per_buffer_;
+
+  // Maps which isolates have had a grow interrupt handled on the buffer. This
+  // is maintained to ensure that the instances are updated with the right size
+  // on Grow.
+  std::unordered_map<const void*, std::unordered_set<Isolate*>>
+      isolates_updated_on_grow_;
+
+  // Maps backing stores(void*) to the size of the underlying memory in
+  // (size_t). An entry to this map is made on a grow call to the corresponding
+  // backing store. On consecutive grow calls to the same backing store,
+  // the size entry is updated. This entry is made right after the mprotect
+  // call to change the protections on a backing_store, so the memory objects
+  // have not been updated yet. The backing store entry in this map is erased
+  // when all the memory objects, or instances that share this backing store
+  // have their bounds updated.
+  std::unordered_map<void*, size_t> grow_update_map_;
+
+  // End of fields protected by {mutex_}.
+  //////////////////////////////////////////////////////////////////////////////
+
   DISALLOW_COPY_AND_ASSIGN(WasmMemoryTracker);
 };
 

src/wasm/wasm-objects.cc

@@ -910,12 +910,30 @@ void WasmTableObject::ClearDispatchTables(Isolate* isolate,
 }
 
 namespace {
+bool AdjustBufferPermissions(Isolate* isolate, Handle<JSArrayBuffer> old_buffer,
+                             size_t new_size) {
+  if (new_size > old_buffer->allocation_length()) return false;
+  void* old_mem_start = old_buffer->backing_store();
+  size_t old_size = old_buffer->byte_length();
+  if (old_size != new_size) {
+    DCHECK_NOT_NULL(old_mem_start);
+    DCHECK_GE(new_size, old_size);
+    // If adjusting permissions fails, propagate error back to return
+    // failure to grow.
+    if (!i::SetPermissions(GetPlatformPageAllocator(), old_mem_start, new_size,
+                           PageAllocator::kReadWrite)) {
+      return false;
+    }
+    reinterpret_cast<v8::Isolate*>(isolate)
+        ->AdjustAmountOfExternalAllocatedMemory(new_size - old_size);
+  }
+  return true;
+}
+
 MaybeHandle<JSArrayBuffer> MemoryGrowBuffer(Isolate* isolate,
                                             Handle<JSArrayBuffer> old_buffer,
                                             size_t new_size) {
   CHECK_EQ(0, new_size % wasm::kWasmPageSize);
-  size_t old_size = old_buffer->byte_length();
-  void* old_mem_start = old_buffer->backing_store();
   // Reusing the backing store from externalized buffers causes problems with
   // Blink's array buffers. The connection between the two is lost, which can
   // lead to Blink not knowing about the other reference to the buffer and
@@ -932,10 +950,12 @@ MaybeHandle<JSArrayBuffer> MemoryGrowBuffer(Isolate* isolate,
     // If the old buffer had full guard regions, we can only safely use the new
     // buffer if it also has full guard regions. Otherwise, we'd have to
     // recompile all the instances using this memory to insert bounds checks.
+    void* old_mem_start = old_buffer->backing_store();
     if (memory_tracker->HasFullGuardRegions(old_mem_start) &&
         !memory_tracker->HasFullGuardRegions(new_buffer->backing_store())) {
       return {};
     }
+    size_t old_size = old_buffer->byte_length();
     if (old_size == 0) return new_buffer;
     memcpy(new_buffer->backing_store(), old_mem_start, old_size);
     DCHECK(old_buffer.is_null() || !old_buffer->is_shared());
@@ -943,18 +963,7 @@ MaybeHandle<JSArrayBuffer> MemoryGrowBuffer(Isolate* isolate,
     i::wasm::DetachMemoryBuffer(isolate, old_buffer, free_memory);
     return new_buffer;
   } else {
-    if (old_size != new_size) {
-      DCHECK_NOT_NULL(old_buffer->backing_store());
-      // If adjusting permissions fails, propagate error back to return
-      // failure to grow.
-      if (!i::SetPermissions(GetPlatformPageAllocator(), old_mem_start,
-                             new_size, PageAllocator::kReadWrite)) {
-        return {};
-      }
-      DCHECK_GE(new_size, old_size);
-      reinterpret_cast<v8::Isolate*>(isolate)
-          ->AdjustAmountOfExternalAllocatedMemory(new_size - old_size);
-    }
+    if (!AdjustBufferPermissions(isolate, old_buffer, new_size)) return {};
     // NOTE: We must allocate a new array buffer here because the spec
     // assumes that ArrayBuffers do not change size.
     void* backing_store = old_buffer->backing_store();
@@ -1074,15 +1083,32 @@ void WasmMemoryObject::AddInstance(Isolate* isolate,
   SetInstanceMemory(instance, buffer);
 }
 
+void WasmMemoryObject::update_instances(Isolate* isolate,
+                                        Handle<JSArrayBuffer> buffer) {
+  if (has_instances()) {
+    Handle<WeakArrayList> instances(this->instances(), isolate);
+    for (int i = 0; i < instances->length(); i++) {
+      MaybeObject elem = instances->Get(i);
+      HeapObject heap_object;
+      if (elem->GetHeapObjectIfWeak(&heap_object)) {
+        Handle<WasmInstanceObject> instance(
+            WasmInstanceObject::cast(heap_object), isolate);
+        SetInstanceMemory(instance, buffer);
+      } else {
+        DCHECK(elem->IsCleared());
+      }
+    }
+  }
+  set_array_buffer(*buffer);
+}
+
 // static
 int32_t WasmMemoryObject::Grow(Isolate* isolate,
                                Handle<WasmMemoryObject> memory_object,
                                uint32_t pages) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.wasm"), "GrowMemory");
   Handle<JSArrayBuffer> old_buffer(memory_object->array_buffer(), isolate);
-  // TODO(gdeepti): Remove check for is_shared when Growing Shared memory
-  // is supported.
-  if (!old_buffer->is_growable() || old_buffer->is_shared()) return -1;
+  if (!old_buffer->is_growable()) return -1;
 
   // Checks for maximum memory size, compute new size.
   uint32_t maximum_pages = wasm::max_mem_pages();
@@ -1102,28 +1128,45 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
   size_t new_size =
       static_cast<size_t>(old_pages + pages) * wasm::kWasmPageSize;
 
-  // Grow the buffer.
+  // Memory is grown, but the memory objects and instances are not yet updated.
+  // Handle this in the interrupt handler so that it's safe for all the isolates
+  // that share this buffer to be updated safely.
   Handle<JSArrayBuffer> new_buffer;
-  if (!MemoryGrowBuffer(isolate, old_buffer, new_size).ToHandle(&new_buffer)) {
-    return -1;
-  }
-
-  // Update instances if any.
-  if (memory_object->has_instances()) {
-    Handle<WeakArrayList> instances(memory_object->instances(), isolate);
-    for (int i = 0; i < instances->length(); i++) {
-      MaybeObject elem = instances->Get(i);
-      HeapObject heap_object;
-      if (elem->GetHeapObjectIfWeak(&heap_object)) {
-        Handle<WasmInstanceObject> instance(
-            WasmInstanceObject::cast(heap_object), isolate);
-        SetInstanceMemory(instance, new_buffer);
-      } else {
-        DCHECK(elem->IsCleared());
-      }
+  if (old_buffer->is_shared()) {
+    // Adjust protections for the buffer.
+    if (!AdjustBufferPermissions(isolate, old_buffer, new_size)) {
+      return -1;
+    }
+    wasm::WasmMemoryTracker* const memory_tracker =
+        isolate->wasm_engine()->memory_tracker();
+    void* backing_store = old_buffer->backing_store();
+    if (memory_tracker->IsWasmSharedMemory(backing_store)) {
+      // This memory is shared between different isolates.
+      DCHECK(old_buffer->is_shared());
+      // Update pending grow state, and trigger a grow interrupt on all the
+      // isolates that share this buffer.
+      isolate->wasm_engine()->memory_tracker()->SetPendingUpdateOnGrow(
+          old_buffer, new_size);
+      // Handle interrupts for this isolate so that the instances with this
+      // isolate are updated.
+      isolate->stack_guard()->HandleInterrupts();
+      // Failure to allocate, or adjust pemissions already handled here, and
+      // updates to instances handled in the interrupt handler safe to return.
+      return static_cast<uint32_t>(old_size / wasm::kWasmPageSize);
+    }
+    // SharedArrayBuffer, but not shared across isolates. Setup a new buffer
+    // with updated permissions and update the instances.
+    new_buffer = wasm::SetupArrayBuffer(isolate, backing_store, new_size,
+                                        old_buffer->is_external());
+    memory_object->update_instances(isolate, new_buffer);
+  } else {
+    if (!MemoryGrowBuffer(isolate, old_buffer, new_size)
+             .ToHandle(&new_buffer)) {
+      return -1;
     }
   }
-  memory_object->set_array_buffer(*new_buffer);
+  // Update instances if any.
+  memory_object->update_instances(isolate, new_buffer);
   return static_cast<uint32_t>(old_size / wasm::kWasmPageSize);
 }
 

src/wasm/wasm-objects.h

@@ -330,6 +330,8 @@ class WasmMemoryObject : public JSObject {
       Isolate* isolate, uint32_t initial, uint32_t maximum,
       bool is_shared_memory);
 
+  void update_instances(Isolate* isolate, Handle<JSArrayBuffer> buffer);
+
   static int32_t Grow(Isolate*, Handle<WasmMemoryObject>, uint32_t pages);
 
   OBJECT_CONSTRUCTORS(WasmMemoryObject, JSObject);