From 45ce1fda882b0c925e36983609e45907a4c3d8bb Mon Sep 17 00:00:00 2001 From: Austin Eng Date: Fri, 22 Jan 2021 00:25:05 +0000 Subject: [PATCH] dawn_wire: Add an API to reclaim reserved devices and textures Dawn Wire has a way to reserve an ID and generation on the client side, but if these reservations are never injected on the server, then it will be impossible to reclaim the in-use ObjectIDs. Bug: dawn:565 Change-Id: I751fce237c881e8cbdeaba18ad0ec1e124bd7ac2 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38281 Commit-Queue: Austin Eng Reviewed-by: Corentin Wallez Reviewed-by: Stephen White --- src/dawn_wire/WireClient.cpp | 8 ++++++ src/dawn_wire/client/Client.cpp | 8 ++++++ src/dawn_wire/client/Client.h | 3 +++ src/include/dawn_wire/WireClient.h | 3 +++ .../unittests/wire/WireInjectDeviceTests.cpp | 26 +++++++++++++++++++ .../unittests/wire/WireInjectTextureTests.cpp | 26 +++++++++++++++++++ 6 files changed, 74 insertions(+) diff --git a/src/dawn_wire/WireClient.cpp b/src/dawn_wire/WireClient.cpp index 0dcea37306..d20fdc9239 100644 --- a/src/dawn_wire/WireClient.cpp +++ b/src/dawn_wire/WireClient.cpp @@ -41,6 +41,14 @@ namespace dawn_wire { return mImpl->ReserveDevice(); } + void WireClient::ReclaimTextureReservation(const ReservedTexture& reservation) { + mImpl->ReclaimTextureReservation(reservation); + } + + void WireClient::ReclaimDeviceReservation(const ReservedDevice& reservation) { + mImpl->ReclaimDeviceReservation(reservation); + } + void WireClient::Disconnect() { mImpl->Disconnect(); } diff --git a/src/dawn_wire/client/Client.cpp b/src/dawn_wire/client/Client.cpp index b5665293f9..7665a70f99 100644 --- a/src/dawn_wire/client/Client.cpp +++ b/src/dawn_wire/client/Client.cpp @@ -118,6 +118,14 @@ namespace dawn_wire { namespace client { return result; } + void Client::ReclaimTextureReservation(const ReservedTexture& reservation) { + TextureAllocator().Free(FromAPI(reservation.texture)); + } + + void Client::ReclaimDeviceReservation(const ReservedDevice& reservation) { + DeviceAllocator().Free(FromAPI(reservation.device)); + } + void Client::Disconnect() { mDisconnected = true; mSerializer = ChunkedCommandSerializer(NoopCommandSerializer::GetInstance()); diff --git a/src/dawn_wire/client/Client.h b/src/dawn_wire/client/Client.h index dd7ac76260..d0e3d5adb0 100644 --- a/src/dawn_wire/client/Client.h +++ b/src/dawn_wire/client/Client.h @@ -48,6 +48,9 @@ namespace dawn_wire { namespace client { ReservedTexture ReserveTexture(WGPUDevice device); ReservedDevice ReserveDevice(); + void ReclaimTextureReservation(const ReservedTexture& reservation); + void ReclaimDeviceReservation(const ReservedDevice& reservation); + template void SerializeCommand(const Cmd& cmd) { mSerializer.SerializeCommand(cmd, *this); diff --git a/src/include/dawn_wire/WireClient.h b/src/include/dawn_wire/WireClient.h index b8f12472a3..5f8b4797c2 100644 --- a/src/include/dawn_wire/WireClient.h +++ b/src/include/dawn_wire/WireClient.h @@ -61,6 +61,9 @@ namespace dawn_wire { ReservedTexture ReserveTexture(WGPUDevice device); ReservedDevice ReserveDevice(); + void ReclaimTextureReservation(const ReservedTexture& reservation); + void ReclaimDeviceReservation(const ReservedDevice& reservation); + // Disconnects the client. // Commands allocated after this point will not be sent. void Disconnect(); diff --git a/src/tests/unittests/wire/WireInjectDeviceTests.cpp b/src/tests/unittests/wire/WireInjectDeviceTests.cpp index 897af4da6f..f1cad6dc6f 100644 --- a/src/tests/unittests/wire/WireInjectDeviceTests.cpp +++ b/src/tests/unittests/wire/WireInjectDeviceTests.cpp @@ -228,3 +228,29 @@ TEST_F(WireInjectDeviceTests, TrackChildObjectsWithTwoReservedDevices) { EXPECT_CALL(api, OnDeviceSetUncapturedErrorCallback(serverDevice2, nullptr, nullptr)).Times(1); EXPECT_CALL(api, OnDeviceSetDeviceLostCallback(serverDevice2, nullptr, nullptr)).Times(1); } + +// Test that a device reservation can be reclaimed. This is necessary to +// avoid leaking ObjectIDs for reservations that are never injected. +TEST_F(WireInjectDeviceTests, ReclaimDeviceReservation) { + // Test that doing a reservation and full release is an error. + { + ReservedDevice reservation = GetWireClient()->ReserveDevice(); + wgpuDeviceRelease(reservation.device); + FlushClient(false); + } + + // Test that doing a reservation and then reclaiming it recycles the ID. + { + ReservedDevice reservation1 = GetWireClient()->ReserveDevice(); + GetWireClient()->ReclaimDeviceReservation(reservation1); + + ReservedDevice reservation2 = GetWireClient()->ReserveDevice(); + + // The ID is the same, but the generation is still different. + ASSERT_EQ(reservation1.id, reservation2.id); + ASSERT_NE(reservation1.generation, reservation2.generation); + + // No errors should occur. + FlushClient(); + } +} diff --git a/src/tests/unittests/wire/WireInjectTextureTests.cpp b/src/tests/unittests/wire/WireInjectTextureTests.cpp index c6a1f2cea6..d8df43afd7 100644 --- a/src/tests/unittests/wire/WireInjectTextureTests.cpp +++ b/src/tests/unittests/wire/WireInjectTextureTests.cpp @@ -86,3 +86,29 @@ TEST_F(WireInjectTextureTests, InjectedTextureLifetime) { DeleteServer(); Mock::VerifyAndClearExpectations(&api); } + +// Test that a texture reservation can be reclaimed. This is necessary to +// avoid leaking ObjectIDs for reservations that are never injected. +TEST_F(WireInjectTextureTests, ReclaimTextureReservation) { + // Test that doing a reservation and full release is an error. + { + ReservedTexture reservation = GetWireClient()->ReserveTexture(device); + wgpuTextureRelease(reservation.texture); + FlushClient(false); + } + + // Test that doing a reservation and then reclaiming it recycles the ID. + { + ReservedTexture reservation1 = GetWireClient()->ReserveTexture(device); + GetWireClient()->ReclaimTextureReservation(reservation1); + + ReservedTexture reservation2 = GetWireClient()->ReserveTexture(device); + + // The ID is the same, but the generation is still different. + ASSERT_EQ(reservation1.id, reservation2.id); + ASSERT_NE(reservation1.generation, reservation2.generation); + + // No errors should occur. + FlushClient(); + } +}