diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 390ed576b6..0f86d22e20 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -84,7 +84,7 @@ namespace dawn_native { // DeviceBase DeviceBase::DeviceBase(AdapterBase* adapter, const DeviceDescriptor* descriptor) - : mAdapter(adapter) { + : mInstance(adapter->GetInstance()), mAdapter(adapter) { if (descriptor != nullptr) { ApplyToggleOverrides(descriptor); ApplyExtensions(descriptor); @@ -94,8 +94,7 @@ namespace dawn_native { SetDefaultToggles(); } - DeviceBase::~DeviceBase() { - } + DeviceBase::~DeviceBase() = default; MaybeError DeviceBase::Initialize(QueueBase* defaultQueue) { mDefaultQueue = AcquireRef(defaultQueue); diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index c4a49e1adf..04d1bb01af 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -364,6 +364,11 @@ namespace dawn_native { wgpu::DeviceLostCallback mDeviceLostCallback = nullptr; void* mDeviceLostUserdata = nullptr; + // The Device keeps a ref to the Instance so that any live Device keeps the Instance alive. + // The Instance shouldn't need to ref child objects so this shouldn't introduce ref cycles. + // The Device keeps a simple pointer to the Adapter because the Adapter is owned by the + // Instance. + Ref mInstance; AdapterBase* mAdapter = nullptr; Ref mRootErrorScope; diff --git a/src/tests/BUILD.gn b/src/tests/BUILD.gn index 10ab9c5209..8fec31f556 100644 --- a/src/tests/BUILD.gn +++ b/src/tests/BUILD.gn @@ -287,6 +287,7 @@ source_set("dawn_end2end_tests_sources") { "end2end/DepthStencilSamplingTests.cpp", "end2end/DepthStencilStateTests.cpp", "end2end/DestroyTests.cpp", + "end2end/DeviceInitializationTests.cpp", "end2end/DeviceLostTests.cpp", "end2end/DrawIndexedIndirectTests.cpp", "end2end/DrawIndexedTests.cpp", diff --git a/src/tests/end2end/DeviceInitializationTests.cpp b/src/tests/end2end/DeviceInitializationTests.cpp new file mode 100644 index 0000000000..b90da97e28 --- /dev/null +++ b/src/tests/end2end/DeviceInitializationTests.cpp @@ -0,0 +1,107 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn/dawn_proc.h" +#include "tests/DawnTest.h" +#include "utils/SystemUtils.h" +#include "utils/WGPUHelpers.h" + +class DeviceInitializationTest : public testing::Test { + void SetUp() override { + dawnProcSetProcs(&dawn_native::GetProcs()); + } + + void TearDown() override { + dawnProcSetProcs(nullptr); + } +}; + +// Test that device operations are still valid if the reference to the instance +// is dropped. +TEST_F(DeviceInitializationTest, DeviceOutlivesInstance) { + // Get properties of all available adapters and then free the instance. + // We want to create a device on a fresh instance and adapter each time. + std::vector availableAdapterProperties; + { + auto instance = std::make_unique(); + instance->DiscoverDefaultAdapters(); + for (const dawn_native::Adapter& adapter : instance->GetAdapters()) { + wgpu::AdapterProperties properties; + adapter.GetProperties(&properties); + + if (properties.backendType == wgpu::BackendType::Null) { + continue; + } + availableAdapterProperties.push_back(properties); + } + } + + for (const wgpu::AdapterProperties& desiredProperties : availableAdapterProperties) { + wgpu::Device device; + + auto instance = std::make_unique(); + instance->DiscoverDefaultAdapters(); + for (dawn_native::Adapter& adapter : instance->GetAdapters()) { + wgpu::AdapterProperties properties; + adapter.GetProperties(&properties); + + if (properties.deviceID == desiredProperties.deviceID && + properties.vendorID == desiredProperties.vendorID && + properties.adapterType == desiredProperties.adapterType && + properties.backendType == desiredProperties.backendType) { + // Create the device, destroy the instance, and break out of the loop. + dawn_native::DeviceDescriptor deviceDescriptor = {}; + device = wgpu::Device(adapter.CreateDevice(&deviceDescriptor)); + instance.reset(); + break; + } + } + + // Now, test that the device can still be used by testing a buffer copy. + wgpu::Buffer src = + utils::CreateBufferFromData(device, wgpu::BufferUsage::CopySrc, {1, 2, 3, 4}); + + wgpu::Buffer dst = utils::CreateBufferFromData( + device, wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::MapRead, {0, 0, 0, 0}); + + wgpu::CommandEncoder encoder = device.CreateCommandEncoder(); + encoder.CopyBufferToBuffer(src, 0, dst, 0, 4 * sizeof(uint32_t)); + + wgpu::CommandBuffer commands = encoder.Finish(); + device.GetDefaultQueue().Submit(1, &commands); + + bool done = false; + dst.MapAsync( + wgpu::MapMode::Read, 0, 4 * sizeof(uint32_t), + [](WGPUBufferMapAsyncStatus status, void* userdata) { + EXPECT_EQ(status, WGPUBufferMapAsyncStatus_Success); + *static_cast(userdata) = true; + }, + &done); + + // Note: we can't actually test this if Tick moves over to + // wgpuInstanceProcessEvents. We can still test that object creation works + // without crashing. + while (!done) { + device.Tick(); + utils::USleep(100); + } + + const uint32_t* mapping = static_cast(dst.GetConstMappedRange()); + EXPECT_EQ(mapping[0], 1u); + EXPECT_EQ(mapping[1], 2u); + EXPECT_EQ(mapping[2], 3u); + EXPECT_EQ(mapping[3], 4u); + } +}