commit e5fbd7ea4fe929e6253081aa5c0ab50d0919080c
author Lokbondo Kung <lokokung@google.com> Fri Oct 11 08:07:47 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Oct 11 08:07:47 2024 +0000
tree 0d2de7ea9aaf66a921f02b026f00b3062f2e313b
parent f3c7cc5c580eb743829c78bb77df0c1e8f6a6ce3

[dawn][wire] Fixes last dropped reference for buffer unmap in wire.

- Makes the wire Buffer external refcounted so that when the last
  external reference is dropped, we can immediately resolve all
  mapAsync calls.
- Before this change, dropping the last external reference would
  not immediately trigger the callback.

Change-Id: I416d7ea91f020156bfb4b15b13b0d5d09c3e0aa2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/210135
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Loko Kung <lokokung@google.com>

src/dawn/tests/unittests/validation/BufferValidationTests.cpp

diff --git a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
index 32f6562..92d77ed 100644
--- a/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
+++ b/src/dawn/tests/unittests/validation/BufferValidationTests.cpp
@@ -461,6 +461,18 @@
     WaitForAllOperations();
 }
 
+// Test map async but dropping the last reference before the result is ready.
+TEST_P(BufferMappingValidationTest, MapAsync_DroppedBeforeResult) {
+    wgpu::Buffer buffer = CreateBuffer(4);
+
+    MockMapAsyncCallback mockCb;
+    EXPECT_CALL(mockCb, Call(wgpu::MapAsyncStatus::Aborted, HasSubstr("destroyed"))).Times(1);
+
+    buffer.MapAsync(GetParam(), 0, 4, wgpu::CallbackMode::AllowProcessEvents, mockCb.Callback());
+    buffer = nullptr;
+    WaitForAllOperations();
+}
+
 // Test that the MapCallback isn't fired twice when unmap() is called inside the callback
 TEST_P(BufferMappingValidationTest, MapAsync_UnmapCalledInCallback) {
     wgpu::Buffer buffer = CreateBuffer(4);

src/dawn/wire/client/Buffer.cpp

diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp
index d08710a..e9a50c9 100644
--- a/src/dawn/wire/client/Buffer.cpp
+++ b/src/dawn/wire/client/Buffer.cpp
@@ -201,7 +201,7 @@
 
     std::optional<WGPUBufferMapAsyncStatus> mStatus;
 
-    // Strong reference to the buffer so that when we call the callback we can pass the buffer.
+    // Reference to the buffer to handle the internal states.
     Ref<Buffer> mBuffer;
 };
 
@@ -443,7 +443,7 @@
                const ObjectHandle& eventManagerHandle,
                Device* device,
                const WGPUBufferDescriptor* descriptor)
-    : ObjectWithEventsBase(params, eventManagerHandle),
+    : RefCountedWithExternalCount<ObjectWithEventsBase>(params, eventManagerHandle),
       mSize(descriptor->size),
       mUsage(static_cast<WGPUBufferUsage>(descriptor->usage)),
       // This flag is for the write handle created by mappedAtCreation
@@ -457,6 +457,10 @@
     ObjectWithEventsBase::DeleteThis();
 }
 
+void Buffer::WillDropLastExternalRef() {
+    SetFutureStatus(WGPUBufferMapAsyncStatus_DestroyedBeforeCallback);
+}
+
 ObjectType Buffer::GetObjectType() const {
     return ObjectType::Buffer;
 }

src/dawn/wire/client/Buffer.h

diff --git a/src/dawn/wire/client/Buffer.h b/src/dawn/wire/client/Buffer.h
index 2abb17c..533635d 100644
--- a/src/dawn/wire/client/Buffer.h
+++ b/src/dawn/wire/client/Buffer.h
@@ -35,7 +35,7 @@
 
 #include "dawn/common/FutureUtils.h"
 #include "dawn/common/Ref.h"
-#include "dawn/common/RefCounted.h"
+#include "dawn/common/RefCountedWithExternalCount.h"
 #include "dawn/wire/WireClient.h"
 #include "dawn/wire/client/ObjectBase.h"
 #include "partition_alloc/pointers/raw_ptr.h"
@@ -44,7 +44,7 @@
 
 class Device;
 
-class Buffer final : public ObjectWithEventsBase {
+class Buffer final : public RefCountedWithExternalCount<ObjectWithEventsBase> {
   public:
     static WGPUBuffer Create(Device* device, const WGPUBufferDescriptor* descriptor);
     static WGPUBuffer CreateError(Device* device, const WGPUBufferDescriptor* descriptor);
@@ -87,6 +87,8 @@
     class MapAsyncEvent;
     class MapAsyncEvent2;
 
+    void WillDropLastExternalRef() override;
+
     // Prepares the callbacks to be called and potentially calls them
     void SetFutureStatus(WGPUBufferMapAsyncStatus status);