Bug 1976766 - Stop assuming a buffer is still mapped in the GPU process. r=webgpu-reviewers,ErichDonGubler

`MapCallback`, `BufferUnmap`, `ReadbackPresentCallback` and
`ReadbackSnapshotCallback` all read a buffer's mapped range while
assuming it is still mapped. `MapCallback` is not invoked by wgpu-core
directly but from a task, so content can destroy or unmap the buffer, or
its device, before it runs. And once a device is lost or destroyed
wgpu-core destroys all of its buffers in `release_gpu_resources`, at the
end of whichever poll drains the device's queue; that poll is driven by
the GPU process, with no content involvement and no ordering against
content's messages.

Return a null ptr for `DestroyedResource` and `NotMapped` rather
than panicking, and handle that at each call site: send a map error to
content, skip the write-back flush, or skip the readback. `MapCallback`
also has to tolerate the parent's `mapData` for the buffer being gone,
which can be caused by content destroying or dropping the buffer.

Related fixes:

- moved `MapCallback`'s `get_mapped_range` call out of the read-only
  branch so every map is checked.

- `Buffer::Unmap` previously cancelled a pending map without telling the
  GPU process, so content saw the buffer as unmapped while wgpu-core
  still
  had it mapped. Forwarding that unmap is what makes the `NotMapped`
  race
  above reachable.

Differential Revision: https://phabricator.services.mozilla.com/D324946

dom/webgpu/Buffer.cpp

@@ -406,6 +406,8 @@ void Buffer::Unmap(JSContext* aCx, ErrorResult& aRv) {
   AbortMapRequest();
 
   if (!mMapped) {
+    ffi::wgpu_client_buffer_unmap(GetClient(), mParent->GetId(), GetId(),
+                                  false);
     return;
   }
 

dom/webgpu/ipc/WebGPUParent.cpp

@@ -519,46 +519,63 @@ void WebGPUParent::MapCallback(uint8_t* aUserData,
     return;
   }
 
-  ipc::ByteBuf bb;
-
   if (aStatus != ffi::WGPUBufferMapAsyncStatus_Success) {
     auto error = nsPrintfCString("Mapping WebGPU buffer failed: %s",
                                  MapStatusString(aStatus));
 
-    ffi::wgpu_server_pack_buffer_map_error(req->mBufferId, &error, ToFFI(&bb));
-  } else {
-    auto* mapData = req->mParent->GetBufferMapData(req->mBufferId);
-    MOZ_RELEASE_ASSERT(mapData);
-
-    auto size = req->mSize;
-    auto offset = req->mOffset;
-
-    if (req->mHostMap == ffi::WGPUHostMap_Read && size > 0) {
-      const auto src = ffi::wgpu_server_buffer_get_mapped_range(
-          req->mParent->GetContext(), req->mBufferId, offset, size);
-
-      MOZ_RELEASE_ASSERT(src.ptr != nullptr);
-      MOZ_RELEASE_ASSERT(src.length >= size);
-      auto shmSize = mapData->mShmem->Size();
-      MOZ_RELEASE_ASSERT(offset <= shmSize);
-      MOZ_RELEASE_ASSERT(size <= shmSize - offset);
-
-      auto dst = mapData->mShmem->DataAsSpan<uint8_t>().Subspan(offset, size);
-      memcpy(dst.data(), src.ptr, size);
-    }
-
-    bool is_writable = req->mHostMap == ffi::WGPUHostMap_Write;
-    ffi::wgpu_server_pack_buffer_map_success(req->mBufferId, is_writable,
-                                             offset, size, ToFFI(&bb));
-
-    mapData->mMappedOffset = offset;
-    mapData->mMappedSize = size;
-    mapData->mIsMapped = true;
+    ffi::wgpu_server_send_buffer_map_error(req->mParent, req->mBufferId,
+                                           &error);
+    return;
   }
 
-  if (!req->mParent->SendServerMessage(std::move(bb))) {
-    NS_ERROR("SendServerMessage failed");
+  auto* mapData = req->mParent->GetBufferMapData(req->mBufferId);
+
+  // The buffer mapping callback can race with buffer.destroy()
+  // since the callback is not directly called by wgpu-core; it's
+  // called later by a task on the same thread as the WebGPU parent actor.
+  if (!mapData) {
+    auto error = nsCString("Mapping WebGPU buffer failed: Map aborted");
+    ffi::wgpu_server_send_buffer_map_error(req->mParent, req->mBufferId,
+                                           &error);
+    return;
   }
+
+  auto size = req->mSize;
+  auto offset = req->mOffset;
+
+  const auto src = ffi::wgpu_server_buffer_get_mapped_range(
+      req->mParent->GetContext(), req->mBufferId, offset, size);
+
+  // The buffer mapping callback can race with buffer.destroy() and
+  // buffer.unmap() since the callback is not directly called by wgpu-core;
+  // it's called later by a task on the same thread as the WebGPU parent
+  // actor.
+  if (src.ptr == nullptr && src.length == 0) {
+    auto error = nsCString("Mapping WebGPU buffer failed: Map aborted");
+    ffi::wgpu_server_send_buffer_map_error(req->mParent, req->mBufferId,
+                                           &error);
+    return;
+  }
+
+  MOZ_RELEASE_ASSERT(src.ptr != nullptr);
+  MOZ_RELEASE_ASSERT(src.length >= size);
+
+  if (req->mHostMap == ffi::WGPUHostMap_Read && size > 0) {
+    auto shmSize = mapData->mShmem->Size();
+    MOZ_RELEASE_ASSERT(offset <= shmSize);
+    MOZ_RELEASE_ASSERT(size <= shmSize - offset);
+
+    auto dst = mapData->mShmem->DataAsSpan<uint8_t>().Subspan(offset, size);
+    memcpy(dst.data(), src.ptr, size);
+  }
+
+  mapData->mMappedOffset = offset;
+  mapData->mMappedSize = size;
+  mapData->mIsMapped = true;
+
+  bool is_writable = req->mHostMap == ffi::WGPUHostMap_Write;
+  ffi::wgpu_server_send_buffer_map_success(req->mParent, req->mBufferId,
+                                           is_writable, offset, size);
 }
 
 void WebGPUParent::BufferUnmap(RawId aDeviceId, RawId aBufferId, bool aFlush) {
@@ -577,14 +594,20 @@ void WebGPUParent::BufferUnmap(RawId aDeviceId, RawId aBufferId, bool aFlush) {
     const auto mapped = ffi::wgpu_server_buffer_get_mapped_range(
         mContext.get(), aBufferId, offset, size);
 
-    MOZ_RELEASE_ASSERT(mapped.ptr != nullptr);
-    MOZ_RELEASE_ASSERT(mapped.length >= size);
-    auto shmSize = mapData->mShmem->Size();
-    MOZ_RELEASE_ASSERT(offset <= shmSize);
-    MOZ_RELEASE_ASSERT(size <= shmSize - offset);
+    // There may be nothing left to flush into: once a device is lost or
+    // destroyed, wgpu-core destroys its buffers in `release_gpu_resources`
+    // on poll.
+    bool is_destroyed = mapped.ptr == nullptr && mapped.length == 0;
+    if (!is_destroyed) {
+      MOZ_RELEASE_ASSERT(mapped.ptr != nullptr);
+      MOZ_RELEASE_ASSERT(mapped.length >= size);
+      auto shmSize = mapData->mShmem->Size();
+      MOZ_RELEASE_ASSERT(offset <= shmSize);
+      MOZ_RELEASE_ASSERT(size <= shmSize - offset);
 
-    auto src = mapData->mShmem->DataAsSpan<uint8_t>().Subspan(offset, size);
-    memcpy(mapped.ptr, src.data(), size);
+      auto src = mapData->mShmem->DataAsSpan<uint8_t>().Subspan(offset, size);
+      memcpy(mapped.ptr, src.data(), size);
+    }
   }
 
   ffi::wgpu_server_buffer_unmap(mContext.get(), aBufferId, mapData->mIsMapped);
@@ -829,6 +852,18 @@ static void ReadbackPresentCallback(uint8_t* userdata,
     const auto mapped = ffi::wgpu_server_buffer_get_mapped_range(
         req->mContext, bufferId, 0, bufferSize);
 
+    // There may be nothing to read back: once a device is lost or
+    // destroyed, wgpu-core destroys its buffers in `release_gpu_resources`
+    // on poll.
+    bool is_destroyed = mapped.ptr == nullptr && mapped.length == 0;
+    if (is_destroyed) {
+      MOZ_LOG(sLogger, LogLevel::Info,
+              ("ReadbackPresentCallback for buffer %" PRIu64
+               " skipped: the readback buffer is gone\n",
+               bufferId));
+      return;
+    }
+
     MOZ_RELEASE_ASSERT(mapped.ptr != nullptr);
     MOZ_RELEASE_ASSERT(mapped.length >= bufferSize);
 
@@ -921,6 +956,18 @@ static void ReadbackSnapshotCallback(uint8_t* userdata,
   const auto mapped = ffi::wgpu_server_buffer_get_mapped_range(
       req->mContext, req->mBufferId, 0, bufferSize);
 
+  // There may be nothing to read back: once a device is lost or
+  // destroyed, wgpu-core destroys its buffers in `release_gpu_resources`
+  // on poll.
+  bool is_destroyed = mapped.ptr == nullptr && mapped.length == 0;
+  if (is_destroyed) {
+    MOZ_LOG(sLogger, LogLevel::Info,
+            ("ReadbackSnapshotCallback for buffer %" PRIu64
+             " skipped: the readback buffer is gone\n",
+             req->mBufferId));
+    return;
+  }
+
   MOZ_RELEASE_ASSERT(mapped.ptr != nullptr);
   MOZ_RELEASE_ASSERT(mapped.length >= bufferSize);
 

gfx/wgpu_bindings/src/server.rs

@@ -1063,7 +1063,19 @@ pub unsafe extern "C" fn wgpu_server_buffer_get_mapped_range(
             ptr: ptr.as_ptr(),
             length: len,
         },
-        Err(error) => panic!("{error}"),
+        Err(error) => match error {
+            // The map may have been cancelled before the caller got here:
+            // `buffer.destroy()` destroys the resource and `buffer.unmap()`
+            // returns it to the idle state. Report an empty slice so the
+            // caller can turn it into a map error.
+            BufferAccessError::DestroyedResource(_) | BufferAccessError::NotMapped => {
+                MappedBufferSlice {
+                    ptr: core::ptr::null_mut(),
+                    length: 0,
+                }
+            }
+            _ => panic!("{error}"),
+        },
     }
 }
 
@@ -2537,30 +2549,32 @@ impl Global {
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn wgpu_server_pack_buffer_map_success(
+pub unsafe extern "C" fn wgpu_server_send_buffer_map_success(
+    parent: WebGPUParentPtr,
     buffer_id: id::BufferId,
     is_writable: bool,
     offset: u64,
     size: u64,
-    bb: &mut ByteBuf,
 ) {
     let result = BufferMapResult::Success {
         is_writable,
         offset,
         size,
     };
-    *bb = make_byte_buf(&ServerMessage::BufferMapResponse(buffer_id, result));
+    let mut byte_buf = make_byte_buf(&ServerMessage::BufferMapResponse(buffer_id, result));
+    unsafe { wgpu_parent_send_server_message(parent, &mut byte_buf) };
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn wgpu_server_pack_buffer_map_error(
+pub unsafe extern "C" fn wgpu_server_send_buffer_map_error(
+    parent: WebGPUParentPtr,
     buffer_id: id::BufferId,
     error: &nsACString,
-    bb: &mut ByteBuf,
 ) {
     let error = error.to_utf8();
     let result = BufferMapResult::Error(error);
-    *bb = make_byte_buf(&ServerMessage::BufferMapResponse(buffer_id, result));
+    let mut byte_buf = make_byte_buf(&ServerMessage::BufferMapResponse(buffer_id, result));
+    unsafe { wgpu_parent_send_server_message(parent, &mut byte_buf) };
 }
 
 #[no_mangle]

testing/web-platform/mozilla/meta/webgpu/cts/webgpu/api/validation/buffer/mapping/cts.https.html.ini

@@ -109,7 +109,6 @@
 [cts.https.html?q=webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:*]
   implementation-status: backlog
   [:]
-    expected: FAIL
 
 
 [cts.https.html?q=webgpu:api,validation,buffer,mapping:mapAsync,usage:*]

testing/web-platform/mozilla/meta/webgpu/cts/webgpu/api/validation/buffer/mapping/dedicated.https.html.ini

@@ -109,7 +109,6 @@
 [dedicated.https.html?worker=dedicated&q=webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:*]
   implementation-status: backlog
   [:]
-    expected: FAIL
 
 
 [dedicated.https.html?worker=dedicated&q=webgpu:api,validation,buffer,mapping:mapAsync,usage:*]

testing/web-platform/mozilla/meta/webgpu/cts/webgpu/api/validation/buffer/mapping/service.https.html.ini

@@ -109,7 +109,6 @@
 [service.https.html?worker=service&q=webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:*]
   implementation-status: backlog
   [:]
-    expected: FAIL
 
 
 [service.https.html?worker=service&q=webgpu:api,validation,buffer,mapping:mapAsync,usage:*]

testing/web-platform/mozilla/meta/webgpu/cts/webgpu/api/validation/buffer/mapping/shared.https.html.ini

@@ -109,7 +109,6 @@
 [shared.https.html?worker=shared&q=webgpu:api,validation,buffer,mapping:mapAsync,state,mappingPending:*]
   implementation-status: backlog
   [:]
-    expected: FAIL
 
 
 [shared.https.html?worker=shared&q=webgpu:api,validation,buffer,mapping:mapAsync,usage:*]