diff --git a/generator/templates/dawn/wire/server/ServerHandlers.cpp b/generator/templates/dawn/wire/server/ServerHandlers.cpp index 9a9f05a6b5..fcba3d1035 100644 --- a/generator/templates/dawn/wire/server/ServerHandlers.cpp +++ b/generator/templates/dawn/wire/server/ServerHandlers.cpp @@ -47,7 +47,7 @@ namespace dawn::wire::server { {% set Type = member.handle_type.name.CamelCase() %} {% set name = as_varName(member.name) %} - auto* {{name}}Data = {{Type}}Objects().Allocate(cmd.{{name}}.id); + auto* {{name}}Data = {{Type}}Objects().Allocate(cmd.{{name}}); if ({{name}}Data == nullptr) { return false; } diff --git a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp index 3438d9b3ab..005522f738 100644 --- a/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp +++ b/src/dawn/tests/unittests/wire/WireInjectTextureTests.cpp @@ -72,6 +72,47 @@ TEST_F(WireInjectTextureTests, InjectExistingID) { reservation.deviceGeneration)); } +// Test that injecting the same id without a destroy first fails. +TEST_F(WireInjectTextureTests, ReuseIDAndGeneration) { + // Do this loop multiple times since the first time, we can't test `generation - 1` since + // generation == 0. + ReservedTexture reservation; + WGPUTexture apiTexture = nullptr; + for (int i = 0; i < 2; ++i) { + reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc); + + apiTexture = api.GetNewTexture(); + EXPECT_CALL(api, TextureReference(apiTexture)); + ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id, + reservation.generation, reservation.deviceId, + reservation.deviceGeneration)); + + // Release the texture. It should be possible to reuse the ID now, but not the generation + wgpuTextureRelease(reservation.texture); + EXPECT_CALL(api, TextureRelease(apiTexture)); + FlushClient(); + + // Invalid to inject with the same ID and generation. + ASSERT_FALSE(GetWireServer()->InjectTexture(apiTexture, reservation.id, + reservation.generation, reservation.deviceId, + reservation.deviceGeneration)); + if (i > 0) { + EXPECT_GE(reservation.generation, 1u); + + // Invalid to inject with the same ID and lesser generation. + ASSERT_FALSE(GetWireServer()->InjectTexture( + apiTexture, reservation.id, reservation.generation - 1, reservation.deviceId, + reservation.deviceGeneration)); + } + } + + // Valid to inject with the same ID and greater generation. + EXPECT_CALL(api, TextureReference(apiTexture)); + ASSERT_TRUE(GetWireServer()->InjectTexture(apiTexture, reservation.id, + reservation.generation + 1, reservation.deviceId, + reservation.deviceGeneration)); +} + // Test that the server only borrows the texture and does a single reference-release TEST_F(WireInjectTextureTests, InjectedTextureLifetime) { ReservedTexture reservation = GetWireClient()->ReserveTexture(device, &placeholderDesc); diff --git a/src/dawn/wire/server/ObjectStorage.h b/src/dawn/wire/server/ObjectStorage.h index 1bef0ad844..4c72c65bc3 100644 --- a/src/dawn/wire/server/ObjectStorage.h +++ b/src/dawn/wire/server/ObjectStorage.h @@ -126,8 +126,8 @@ class KnownObjectsBase { // Allocates the data for a given ID and returns it. // Returns nullptr if the ID is already allocated, or too far ahead, or if ID is 0 (ID 0 is // reserved for nullptr). Invalidates all the Data* - Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) { - if (id == 0 || id > mKnown.size()) { + Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) { + if (handle.id == 0 || handle.id > mKnown.size()) { return nullptr; } @@ -135,17 +135,24 @@ class KnownObjectsBase { data.state = state; data.handle = nullptr; - if (id >= mKnown.size()) { + if (handle.id >= mKnown.size()) { mKnown.push_back(std::move(data)); return &mKnown.back(); } - if (mKnown[id].state != AllocationState::Free) { + if (mKnown[handle.id].state != AllocationState::Free) { return nullptr; } - mKnown[id] = std::move(data); - return &mKnown[id]; + // The generation should be strictly increasing. + if (handle.generation <= mKnown[handle.id].generation) { + return nullptr; + } + // update the generation in the slot + data.generation = handle.generation; + + mKnown[handle.id] = std::move(data); + return &mKnown[handle.id]; } // Marks an ID as deallocated @@ -193,8 +200,8 @@ class KnownObjects : public KnownObjectsBase { public: KnownObjects() = default; - Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) { - Data* data = KnownObjectsBase::Allocate(id, state); + Data* Allocate(ObjectHandle handle, AllocationState state = AllocationState::Allocated) { + Data* data = KnownObjectsBase::Allocate(handle, state); AddToKnownSet(data); return data; } diff --git a/src/dawn/wire/server/Server.cpp b/src/dawn/wire/server/Server.cpp index 2796966a4c..7de375f177 100644 --- a/src/dawn/wire/server/Server.cpp +++ b/src/dawn/wire/server/Server.cpp @@ -54,7 +54,7 @@ bool Server::InjectTexture(WGPUTexture texture, return false; } - ObjectData* data = TextureObjects().Allocate(id); + ObjectData* data = TextureObjects().Allocate(ObjectHandle{id, generation}); if (data == nullptr) { return false; } @@ -81,7 +81,7 @@ bool Server::InjectSwapChain(WGPUSwapChain swapchain, return false; } - ObjectData* data = SwapChainObjects().Allocate(id); + ObjectData* data = SwapChainObjects().Allocate(ObjectHandle{id, generation}); if (data == nullptr) { return false; } @@ -99,7 +99,7 @@ bool Server::InjectSwapChain(WGPUSwapChain swapchain, bool Server::InjectDevice(WGPUDevice device, uint32_t id, uint32_t generation) { ASSERT(device != nullptr); - ObjectData* data = DeviceObjects().Allocate(id); + ObjectData* data = DeviceObjects().Allocate(ObjectHandle{id, generation}); if (data == nullptr) { return false; } @@ -121,7 +121,7 @@ bool Server::InjectDevice(WGPUDevice device, uint32_t id, uint32_t generation) { bool Server::InjectInstance(WGPUInstance instance, uint32_t id, uint32_t generation) { ASSERT(instance != nullptr); - ObjectData* data = InstanceObjects().Allocate(id); + ObjectData* data = InstanceObjects().Allocate(ObjectHandle{id, generation}); if (data == nullptr) { return false; } diff --git a/src/dawn/wire/server/ServerAdapter.cpp b/src/dawn/wire/server/ServerAdapter.cpp index 9735d26cd4..8fa9e067ae 100644 --- a/src/dawn/wire/server/ServerAdapter.cpp +++ b/src/dawn/wire/server/ServerAdapter.cpp @@ -28,7 +28,7 @@ bool Server::DoAdapterRequestDevice(ObjectId adapterId, return false; } - auto* resultData = DeviceObjects().Allocate(deviceHandle.id, AllocationState::Reserved); + auto* resultData = DeviceObjects().Allocate(deviceHandle, AllocationState::Reserved); if (resultData == nullptr) { return false; } diff --git a/src/dawn/wire/server/ServerBuffer.cpp b/src/dawn/wire/server/ServerBuffer.cpp index 61d5e15423..f07bdfad77 100644 --- a/src/dawn/wire/server/ServerBuffer.cpp +++ b/src/dawn/wire/server/ServerBuffer.cpp @@ -110,7 +110,7 @@ bool Server::DoDeviceCreateBuffer(ObjectId deviceId, } // Create and register the buffer object. - auto* resultData = BufferObjects().Allocate(bufferResult.id); + auto* resultData = BufferObjects().Allocate(bufferResult); if (resultData == nullptr) { return false; } diff --git a/src/dawn/wire/server/ServerDevice.cpp b/src/dawn/wire/server/ServerDevice.cpp index 27978155a3..0281c3334e 100644 --- a/src/dawn/wire/server/ServerDevice.cpp +++ b/src/dawn/wire/server/ServerDevice.cpp @@ -99,7 +99,7 @@ bool Server::DoDeviceCreateComputePipelineAsync(ObjectId deviceId, } auto* resultData = - ComputePipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved); + ComputePipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved); if (resultData == nullptr) { return false; } @@ -143,7 +143,7 @@ bool Server::DoDeviceCreateRenderPipelineAsync(ObjectId deviceId, } auto* resultData = - RenderPipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved); + RenderPipelineObjects().Allocate(pipelineObjectHandle, AllocationState::Reserved); if (resultData == nullptr) { return false; } diff --git a/src/dawn/wire/server/ServerInstance.cpp b/src/dawn/wire/server/ServerInstance.cpp index dcdf62cc24..4357604375 100644 --- a/src/dawn/wire/server/ServerInstance.cpp +++ b/src/dawn/wire/server/ServerInstance.cpp @@ -29,7 +29,7 @@ bool Server::DoInstanceRequestAdapter(ObjectId instanceId, return false; } - auto* resultData = AdapterObjects().Allocate(adapterHandle.id, AllocationState::Reserved); + auto* resultData = AdapterObjects().Allocate(adapterHandle, AllocationState::Reserved); if (resultData == nullptr) { return false; }