diff --git a/generator/templates/dawn_wire/server/ServerDoers.cpp b/generator/templates/dawn_wire/server/ServerDoers.cpp index a16ff9e665..7a705770d7 100644 --- a/generator/templates/dawn_wire/server/ServerDoers.cpp +++ b/generator/templates/dawn_wire/server/ServerDoers.cpp @@ -82,28 +82,33 @@ namespace dawn_wire { namespace server { return false; } } - {% if type.name.CamelCase() in server_reverse_lookup_objects %} - {{type.name.CamelCase()}}ObjectIdTable().Remove(data->handle); - {% endif %} - {% if type.name.get() == "device" %} - //* TODO(crbug.com/dawn/384): This is a hack to make sure that all child objects - //* are destroyed before their device. We should have a solution in - //* Dawn native that makes all child objects internally null if their - //* Device is destroyed. - while (data->info->childObjectTypesAndIds.size() > 0) { - ObjectType childObjectType; - ObjectId childObjectId; - std::tie(childObjectType, childObjectId) = UnpackObjectTypeAndId( - *data->info->childObjectTypesAndIds.begin()); - DoDestroyObject(childObjectType, childObjectId); - } - if (data->handle != nullptr) { - //* Deregisters uncaptured error and device lost callbacks since - //* they should not be forwarded if the device no longer exists on the wire. - ClearDeviceCallbacks(data->handle); - } - {% endif %} - if (data->handle != nullptr) { + if (data->state == AllocationState::Allocated) { + ASSERT(data->handle != nullptr); + {% if type.name.CamelCase() in server_reverse_lookup_objects %} + {{type.name.CamelCase()}}ObjectIdTable().Remove(data->handle); + {% endif %} + + {% if type.name.get() == "device" %} + //* TODO(crbug.com/dawn/384): This is a hack to make sure that all child objects + //* are destroyed before their device. We should have a solution in + //* Dawn native that makes all child objects internally null if their + //* Device is destroyed. + while (data->info->childObjectTypesAndIds.size() > 0) { + ObjectType childObjectType; + ObjectId childObjectId; + std::tie(childObjectType, childObjectId) = UnpackObjectTypeAndId( + *data->info->childObjectTypesAndIds.begin()); + if (!DoDestroyObject(childObjectType, childObjectId)) { + return false; + } + } + if (data->handle != nullptr) { + //* Deregisters uncaptured error and device lost callbacks since + //* they should not be forwarded if the device no longer exists on the wire. + ClearDeviceCallbacks(data->handle); + } + {% endif %} + mProcs.{{as_varName(type.name, Name("release"))}}(data->handle); } {{type.name.CamelCase()}}Objects().Free(objectId); diff --git a/src/dawn_wire/server/ObjectStorage.h b/src/dawn_wire/server/ObjectStorage.h index bffe8e8b01..d4aa83c224 100644 --- a/src/dawn_wire/server/ObjectStorage.h +++ b/src/dawn_wire/server/ObjectStorage.h @@ -30,15 +30,21 @@ namespace dawn_wire { namespace server { ObjectHandle self; }; + // Whether this object has been allocated, or reserved for async object creation. + // Used by the KnownObjects queries + enum class AllocationState : uint32_t { + Free, + Reserved, + Allocated, + }; + template struct ObjectDataBase { // The backend-provided handle and generation to this object. T handle; uint32_t generation = 0; - // Whether this object has been allocated, used by the KnownObjects queries - // TODO(cwallez@chromium.org): make this an internal bit vector in KnownObjects. - bool allocated; + AllocationState state; // This points to an allocation that is owned by the device. DeviceInfo* deviceInfo = nullptr; @@ -92,33 +98,33 @@ namespace dawn_wire { namespace server { // KnownObjects for ID 0. Data reservation; reservation.handle = nullptr; - reservation.allocated = false; + reservation.state = AllocationState::Free; mKnown.push_back(std::move(reservation)); } // Get a backend objects for a given client ID. // Returns nullptr if the ID hasn't previously been allocated. - const Data* Get(uint32_t id) const { + const Data* Get(uint32_t id, AllocationState expected = AllocationState::Allocated) const { if (id >= mKnown.size()) { return nullptr; } const Data* data = &mKnown[id]; - if (!data->allocated) { + if (data->state != expected) { return nullptr; } return data; } - Data* Get(uint32_t id) { + Data* Get(uint32_t id, AllocationState expected = AllocationState::Allocated) { if (id >= mKnown.size()) { return nullptr; } Data* data = &mKnown[id]; - if (!data->allocated) { + if (data->state != expected) { return nullptr; } @@ -128,13 +134,13 @@ namespace dawn_wire { namespace server { // 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) { + Data* Allocate(uint32_t id, AllocationState state = AllocationState::Allocated) { if (id == 0 || id > mKnown.size()) { return nullptr; } Data data; - data.allocated = true; + data.state = state; data.handle = nullptr; if (id >= mKnown.size()) { @@ -142,7 +148,7 @@ namespace dawn_wire { namespace server { return &mKnown.back(); } - if (mKnown[id].allocated) { + if (mKnown[id].state != AllocationState::Free) { return nullptr; } @@ -153,15 +159,15 @@ namespace dawn_wire { namespace server { // Marks an ID as deallocated void Free(uint32_t id) { ASSERT(id < mKnown.size()); - mKnown[id].allocated = false; + mKnown[id].state = AllocationState::Free; } std::vector AcquireAllHandles() { std::vector objects; for (Data& data : mKnown) { - if (data.allocated && data.handle != nullptr) { + if (data.state == AllocationState::Allocated && data.handle != nullptr) { objects.push_back(data.handle); - data.allocated = false; + data.state = AllocationState::Free; data.handle = nullptr; } } @@ -172,7 +178,7 @@ namespace dawn_wire { namespace server { std::vector GetAllHandles() { std::vector objects; for (Data& data : mKnown) { - if (data.allocated && data.handle != nullptr) { + if (data.state == AllocationState::Allocated && data.handle != nullptr) { objects.push_back(data.handle); } } diff --git a/src/dawn_wire/server/Server.cpp b/src/dawn_wire/server/Server.cpp index 8c97bfc272..5da35fe31c 100644 --- a/src/dawn_wire/server/Server.cpp +++ b/src/dawn_wire/server/Server.cpp @@ -66,7 +66,7 @@ namespace dawn_wire { namespace server { data->handle = texture; data->generation = generation; - data->allocated = true; + data->state = AllocationState::Allocated; data->deviceInfo = device->info.get(); if (!TrackDeviceChild(data->deviceInfo, ObjectType::Texture, id)) { @@ -89,7 +89,7 @@ namespace dawn_wire { namespace server { data->handle = device; data->generation = generation; - data->allocated = true; + data->state = AllocationState::Allocated; data->info->server = this; data->info->self = ObjectHandle{id, generation}; diff --git a/src/dawn_wire/server/ServerDevice.cpp b/src/dawn_wire/server/ServerDevice.cpp index dc0b86b5c3..bcc8db67f2 100644 --- a/src/dawn_wire/server/ServerDevice.cpp +++ b/src/dawn_wire/server/ServerDevice.cpp @@ -22,21 +22,29 @@ namespace dawn_wire { namespace server { void HandleCreateReadyRenderPipelineCallbackResult(KnownObjects* knownObjects, WGPUCreateReadyPipelineStatus status, Pipeline pipeline, - const char* message, CreateReadyPipelineUserData* data) { - auto* pipelineObject = knownObjects->Get(data->pipelineObjectID); + // May be null if the device was destroyed. Device destruction destroys child + // objects on the wire. + auto* pipelineObject = + knownObjects->Get(data->pipelineObjectID, AllocationState::Reserved); + // Should be impossible to fail. ObjectIds can't be freed by a destroy command until + // they move from Reserved to Allocated, or if they are destroyed here. + ASSERT(pipelineObject != nullptr); if (status == WGPUCreateReadyPipelineStatus_Success) { - ASSERT(pipelineObject != nullptr); + // Assign the handle and allocated status if the pipeline is created successfully. + pipelineObject->state = AllocationState::Allocated; pipelineObject->handle = pipeline; - } else if (pipelineObject != nullptr) { - // May be null if the device was destroyed. Device destruction destroys child - // objects on the wire. - if (!UntrackDeviceChild(pipelineObject->deviceInfo, objectType, - data->pipelineObjectID)) { - UNREACHABLE(); - } + + // This should be impossible to fail. It would require a command to be sent that + // creates a duplicate ObjectId, which would fail validation. + bool success = TrackDeviceChild(pipelineObject->deviceInfo, objectType, + data->pipelineObjectID); + ASSERT(success); + } else { + // Otherwise, free the ObjectId which will make it unusable. knownObjects->Free(data->pipelineObjectID); + ASSERT(pipeline == nullptr); } } @@ -103,17 +111,14 @@ namespace dawn_wire { namespace server { return false; } - auto* resultData = ComputePipelineObjects().Allocate(pipelineObjectHandle.id); + auto* resultData = + ComputePipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved); if (resultData == nullptr) { return false; } resultData->generation = pipelineObjectHandle.generation; resultData->deviceInfo = device->info.get(); - if (!TrackDeviceChild(resultData->deviceInfo, ObjectType::ComputePipeline, - pipelineObjectHandle.id)) { - return false; - } auto userdata = MakeUserdata(); userdata->device = ObjectHandle{deviceId, device->generation}; @@ -133,7 +138,7 @@ namespace dawn_wire { namespace server { const char* message, CreateReadyPipelineUserData* data) { HandleCreateReadyRenderPipelineCallbackResult( - &ComputePipelineObjects(), status, pipeline, message, data); + &ComputePipelineObjects(), status, pipeline, data); ReturnDeviceCreateReadyComputePipelineCallbackCmd cmd; cmd.device = data->device; @@ -153,17 +158,14 @@ namespace dawn_wire { namespace server { return false; } - auto* resultData = RenderPipelineObjects().Allocate(pipelineObjectHandle.id); + auto* resultData = + RenderPipelineObjects().Allocate(pipelineObjectHandle.id, AllocationState::Reserved); if (resultData == nullptr) { return false; } resultData->generation = pipelineObjectHandle.generation; resultData->deviceInfo = device->info.get(); - if (!TrackDeviceChild(resultData->deviceInfo, ObjectType::RenderPipeline, - pipelineObjectHandle.id)) { - return false; - } auto userdata = MakeUserdata(); userdata->device = ObjectHandle{deviceId, device->generation}; @@ -183,7 +185,7 @@ namespace dawn_wire { namespace server { const char* message, CreateReadyPipelineUserData* data) { HandleCreateReadyRenderPipelineCallbackResult( - &RenderPipelineObjects(), status, pipeline, message, data); + &RenderPipelineObjects(), status, pipeline, data); ReturnDeviceCreateReadyRenderPipelineCallbackCmd cmd; cmd.device = data->device;