dawn_wire: Skip device inject error if the client drops the device
If the client drops the last reference to the device, it would
dereference an invalid pointer upon calling InjectError. So, skip the
call. We can't keep the device alive if the Buffer is still alive
because we intend to make all objects internally null if you delete
their device.
It is ok to skip error injection because if the client deletes the
device, it should not expect to receive any more error callbacks.
Bug: dawn:384
Change-Id: I4c694310e4395b06cd49603fc5d4cd846799decb
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/37580
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_wire/client/Buffer.cpp b/src/dawn_wire/client/Buffer.cpp
index 40b5759..da3691c 100644
--- a/src/dawn_wire/client/Buffer.cpp
+++ b/src/dawn_wire/client/Buffer.cpp
@@ -63,6 +63,7 @@
auto* bufferObjectAndSerial = wireClient->BufferAllocator().New(wireClient);
Buffer* buffer = bufferObjectAndSerial->object.get();
buffer->mDevice = device;
+ buffer->mDeviceIsAlive = device->GetAliveWeakPtr();
buffer->mSize = descriptor->size;
DeviceCreateBufferCmd cmd;
@@ -92,6 +93,7 @@
WGPUBuffer Buffer::CreateError(Device* device) {
auto* allocation = device->client->BufferAllocator().New(device->client);
allocation->object->mDevice = device;
+ allocation->object->mDeviceIsAlive = device->GetAliveWeakPtr();
DeviceCreateErrorBufferCmd cmd;
cmd.self = ToAPI(device);
@@ -140,8 +142,10 @@
// Step 1. Do early validation of READ ^ WRITE because the server rejects mode = 0.
if (!(isReadMode ^ isWriteMode)) {
- mDevice->InjectError(WGPUErrorType_Validation,
- "MapAsync mode must be exactly one of Read or Write");
+ if (!mDeviceIsAlive.expired()) {
+ mDevice->InjectError(WGPUErrorType_Validation,
+ "MapAsync mode must be exactly one of Read or Write");
+ }
if (callback != nullptr) {
callback(WGPUBufferMapAsyncStatus_Error, userdata);
}
@@ -163,7 +167,10 @@
if (isReadMode) {
request.readHandle.reset(client->GetMemoryTransferService()->CreateReadHandle(size));
if (request.readHandle == nullptr) {
- mDevice->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
+ if (!mDeviceIsAlive.expired()) {
+ mDevice->InjectError(WGPUErrorType_OutOfMemory,
+ "Failed to create buffer mapping");
+ }
callback(WGPUBufferMapAsyncStatus_Error, userdata);
return;
}
@@ -171,7 +178,10 @@
ASSERT(isWriteMode);
request.writeHandle.reset(client->GetMemoryTransferService()->CreateWriteHandle(size));
if (request.writeHandle == nullptr) {
- mDevice->InjectError(WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
+ if (!mDeviceIsAlive.expired()) {
+ mDevice->InjectError(WGPUErrorType_OutOfMemory,
+ "Failed to create buffer mapping");
+ }
callback(WGPUBufferMapAsyncStatus_Error, userdata);
return;
}
diff --git a/src/dawn_wire/client/Buffer.h b/src/dawn_wire/client/Buffer.h
index d6a275c..5e0d5ec 100644
--- a/src/dawn_wire/client/Buffer.h
+++ b/src/dawn_wire/client/Buffer.h
@@ -89,6 +89,8 @@
void* mMappedData = nullptr;
size_t mMapOffset = 0;
size_t mMapSize = 0;
+
+ std::weak_ptr<bool> mDeviceIsAlive;
};
}} // namespace dawn_wire::client
diff --git a/src/dawn_wire/client/Device.cpp b/src/dawn_wire/client/Device.cpp
index 08a3e33..b6ee4fc 100644
--- a/src/dawn_wire/client/Device.cpp
+++ b/src/dawn_wire/client/Device.cpp
@@ -23,7 +23,7 @@
namespace dawn_wire { namespace client {
Device::Device(Client* clientIn, uint32_t initialRefcount, uint32_t initialId)
- : ObjectBase(clientIn, initialRefcount, initialId) {
+ : ObjectBase(clientIn, initialRefcount, initialId), mIsAlive(std::make_shared<bool>()) {
#if defined(DAWN_ENABLE_ASSERTS)
mErrorCallback = [](WGPUErrorType, char const*, void*) {
static bool calledOnce = false;
@@ -114,6 +114,10 @@
mErrorScopes.clear();
}
+ std::weak_ptr<bool> Device::GetAliveWeakPtr() {
+ return mIsAlive;
+ }
+
void Device::SetUncapturedErrorCallback(WGPUErrorCallback errorCallback, void* errorUserdata) {
mErrorCallback = errorCallback;
mErrorUserdata = errorUserdata;
diff --git a/src/dawn_wire/client/Device.h b/src/dawn_wire/client/Device.h
index 6862bb3..3f16700 100644
--- a/src/dawn_wire/client/Device.h
+++ b/src/dawn_wire/client/Device.h
@@ -64,6 +64,8 @@
void CancelCallbacksForDisconnect() override;
+ std::weak_ptr<bool> GetAliveWeakPtr();
+
private:
struct ErrorScopeData {
WGPUErrorCallback callback = nullptr;
@@ -89,6 +91,8 @@
void* mDeviceLostUserdata = nullptr;
Queue* mDefaultQueue = nullptr;
+
+ std::shared_ptr<bool> mIsAlive;
};
}} // namespace dawn_wire::client
diff --git a/src/dawn_wire/client/Fence.cpp b/src/dawn_wire/client/Fence.cpp
index 9bb40b3..d9a858a 100644
--- a/src/dawn_wire/client/Fence.cpp
+++ b/src/dawn_wire/client/Fence.cpp
@@ -38,9 +38,7 @@
mOnCompletionRequests.clear();
}
- void Fence::Initialize(Queue* queue, const WGPUFenceDescriptor* descriptor) {
- mQueue = queue;
-
+ void Fence::Initialize(const WGPUFenceDescriptor* descriptor) {
mCompletedValue = descriptor != nullptr ? descriptor->initialValue : 0u;
}
@@ -87,8 +85,4 @@
return mCompletedValue;
}
- Queue* Fence::GetQueue() const {
- return mQueue;
- }
-
}} // namespace dawn_wire::client
diff --git a/src/dawn_wire/client/Fence.h b/src/dawn_wire/client/Fence.h
index a8fb5f4..7c09948 100644
--- a/src/dawn_wire/client/Fence.h
+++ b/src/dawn_wire/client/Fence.h
@@ -27,7 +27,7 @@
public:
using ObjectBase::ObjectBase;
~Fence();
- void Initialize(Queue* queue, const WGPUFenceDescriptor* descriptor);
+ void Initialize(const WGPUFenceDescriptor* descriptor);
void CheckPassedFences();
void OnCompletion(uint64_t value, WGPUFenceOnCompletionCallback callback, void* userdata);
@@ -35,7 +35,6 @@
bool OnCompletionCallback(uint64_t requestSerial, WGPUFenceCompletionStatus status);
uint64_t GetCompletedValue() const;
- Queue* GetQueue() const;
private:
void CancelCallbacksForDisconnect() override;
@@ -44,7 +43,6 @@
WGPUFenceOnCompletionCallback callback = nullptr;
void* userdata = nullptr;
};
- Queue* mQueue = nullptr;
uint64_t mCompletedValue = 0;
uint64_t mOnCompletionRequestSerial = 0;
std::map<uint64_t, OnCompletionData> mOnCompletionRequests;
diff --git a/src/dawn_wire/client/Queue.cpp b/src/dawn_wire/client/Queue.cpp
index 37ae3fa..f5f68f7 100644
--- a/src/dawn_wire/client/Queue.cpp
+++ b/src/dawn_wire/client/Queue.cpp
@@ -29,7 +29,7 @@
client->SerializeCommand(cmd);
Fence* fence = allocation->object.get();
- fence->Initialize(this, descriptor);
+ fence->Initialize(descriptor);
return ToAPI(fence);
}
diff --git a/src/tests/unittests/wire/WireDestroyObjectTests.cpp b/src/tests/unittests/wire/WireDestroyObjectTests.cpp
index f5e16b7..34b976d 100644
--- a/src/tests/unittests/wire/WireDestroyObjectTests.cpp
+++ b/src/tests/unittests/wire/WireDestroyObjectTests.cpp
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include "tests/MockCallback.h"
#include "tests/unittests/wire/WireTest.h"
using namespace testing;
@@ -43,3 +44,46 @@
wgpuCommandEncoderFinish(encoder, nullptr);
FlushClient(false);
}
+
+// Test that calling a function that would generate an InjectError doesn't crash after
+// the device is destroyed.
+TEST_F(WireDestroyObjectTests, ImplicitInjectErrorAfterDestroyDevice) {
+ WGPUBufferDescriptor bufferDesc = {};
+ bufferDesc.size = 4;
+ WGPUBuffer buffer = wgpuDeviceCreateBuffer(device, &bufferDesc);
+
+ WGPUBuffer apiBuffer = api.GetNewBuffer();
+ EXPECT_CALL(api, DeviceCreateBuffer(apiDevice, _)).WillOnce(Return(apiBuffer));
+
+ FlushClient();
+
+ {
+ // Control case: MapAsync errors on invalid WGPUMapMode.
+ MockCallback<WGPUBufferMapCallback> mockBufferMapCallback;
+
+ EXPECT_CALL(api, DeviceInjectError(apiDevice, WGPUErrorType_Validation, _));
+ EXPECT_CALL(mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, this));
+ wgpuBufferMapAsync(buffer, WGPUMapMode(0), 0, 4, mockBufferMapCallback.Callback(),
+ mockBufferMapCallback.MakeUserdata(this));
+
+ FlushClient();
+ }
+
+ {
+ // Now, release the device. InjectError shouldn't happen.
+ wgpuDeviceRelease(device);
+ MockCallback<WGPUBufferMapCallback> mockBufferMapCallback;
+
+ EXPECT_CALL(mockBufferMapCallback, Call(WGPUBufferMapAsyncStatus_Error, this + 1));
+ wgpuBufferMapAsync(buffer, WGPUMapMode(0), 0, 4, mockBufferMapCallback.Callback(),
+ mockBufferMapCallback.MakeUserdata(this + 1));
+
+ Sequence s1, s2;
+ // The device and child objects alre also released.
+ EXPECT_CALL(api, BufferRelease(apiBuffer)).InSequence(s1);
+ EXPECT_CALL(api, QueueRelease(apiQueue)).InSequence(s2);
+ EXPECT_CALL(api, DeviceRelease(apiDevice)).InSequence(s1, s2);
+
+ FlushClient();
+ }
+}
