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>
This commit is contained in:
parent
3639186e7e
commit
8c58491d25
src
dawn_wire/client
tests/unittests/wire
|
@ -63,6 +63,7 @@ namespace dawn_wire { namespace client {
|
|||
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 @@ namespace dawn_wire { namespace client {
|
|||
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 @@ namespace dawn_wire { namespace client {
|
|||
|
||||
// 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 @@ namespace dawn_wire { namespace client {
|
|||
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 @@ namespace dawn_wire { namespace client {
|
|||
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;
|
||||
}
|
||||
|
|
|
@ -89,6 +89,8 @@ namespace dawn_wire { namespace client {
|
|||
void* mMappedData = nullptr;
|
||||
size_t mMapOffset = 0;
|
||||
size_t mMapSize = 0;
|
||||
|
||||
std::weak_ptr<bool> mDeviceIsAlive;
|
||||
};
|
||||
|
||||
}} // namespace dawn_wire::client
|
||||
|
|
|
@ -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 @@ namespace dawn_wire { namespace client {
|
|||
mErrorScopes.clear();
|
||||
}
|
||||
|
||||
std::weak_ptr<bool> Device::GetAliveWeakPtr() {
|
||||
return mIsAlive;
|
||||
}
|
||||
|
||||
void Device::SetUncapturedErrorCallback(WGPUErrorCallback errorCallback, void* errorUserdata) {
|
||||
mErrorCallback = errorCallback;
|
||||
mErrorUserdata = errorUserdata;
|
||||
|
|
|
@ -64,6 +64,8 @@ namespace dawn_wire { namespace client {
|
|||
|
||||
void CancelCallbacksForDisconnect() override;
|
||||
|
||||
std::weak_ptr<bool> GetAliveWeakPtr();
|
||||
|
||||
private:
|
||||
struct ErrorScopeData {
|
||||
WGPUErrorCallback callback = nullptr;
|
||||
|
@ -89,6 +91,8 @@ namespace dawn_wire { namespace client {
|
|||
void* mDeviceLostUserdata = nullptr;
|
||||
|
||||
Queue* mDefaultQueue = nullptr;
|
||||
|
||||
std::shared_ptr<bool> mIsAlive;
|
||||
};
|
||||
|
||||
}} // namespace dawn_wire::client
|
||||
|
|
|
@ -38,9 +38,7 @@ namespace dawn_wire { namespace client {
|
|||
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 @@ namespace dawn_wire { namespace client {
|
|||
return mCompletedValue;
|
||||
}
|
||||
|
||||
Queue* Fence::GetQueue() const {
|
||||
return mQueue;
|
||||
}
|
||||
|
||||
}} // namespace dawn_wire::client
|
||||
|
|
|
@ -27,7 +27,7 @@ namespace dawn_wire { namespace client {
|
|||
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 @@ namespace dawn_wire { namespace client {
|
|||
bool OnCompletionCallback(uint64_t requestSerial, WGPUFenceCompletionStatus status);
|
||||
|
||||
uint64_t GetCompletedValue() const;
|
||||
Queue* GetQueue() const;
|
||||
|
||||
private:
|
||||
void CancelCallbacksForDisconnect() override;
|
||||
|
@ -44,7 +43,6 @@ namespace dawn_wire { namespace client {
|
|||
WGPUFenceOnCompletionCallback callback = nullptr;
|
||||
void* userdata = nullptr;
|
||||
};
|
||||
Queue* mQueue = nullptr;
|
||||
uint64_t mCompletedValue = 0;
|
||||
uint64_t mOnCompletionRequestSerial = 0;
|
||||
std::map<uint64_t, OnCompletionData> mOnCompletionRequests;
|
||||
|
|
|
@ -29,7 +29,7 @@ namespace dawn_wire { namespace client {
|
|||
client->SerializeCommand(cmd);
|
||||
|
||||
Fence* fence = allocation->object.get();
|
||||
fence->Initialize(this, descriptor);
|
||||
fence->Initialize(descriptor);
|
||||
return ToAPI(fence);
|
||||
}
|
||||
|
||||
|
|
|
@ -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 @@ TEST_F(WireDestroyObjectTests, DestroyDeviceDestroysChildren) {
|
|||
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();
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue