Commit 86511b98 authored by Jiawei Shao's avatar Jiawei Shao Committed by Commit Bot

WebGPU: Don't destroy WebGPUSerializer in the destructor of GPUDevice

This patch intends to fix a crash issue when GPUDevice is destroyed
before the destruction of any other WebGPU objects that are created
from it.

Without this patch, when a GPUDevice is destroyed its related
WebGPUSerializer is also destroyed immediately, which causes crash
when any other WebGPU object which belongs to this GPUDevice is
destroyed afterwards.

In this patch, we introduce DawnDeviceClientSerializerHolder to
manage the reference count of all the WebGPU objects that belong to
one device_client_id (relates to one WebGPUSerializer).
- When GPUDevice is created,
  scopted_ptr<DawnDeviceClientSerializerHolder> is also created with
  dawn_control_client and device_client_id.
- When other WebGPU objects are created from a GPUDevice, they will all
  get a reference of scopted_ptr<DawnDeviceClientSerializerHolder> from
  the GPUDevice
- The WebGPUSerializer will only be released in the destructor of
  scopted_ptr<DawnDeviceClientSerializerHolder> (that should be the
  time when the GPUDevice and all the WebGPU objects that are created
  from it are released).

Note that scoped_refptr<DawnControlClientHolder> is referenced twice in
GPUDevice and other WebGPU objects that are created from it as they are
still inherited from DawnObjectBase. We will fix this issue in the next
patch.

BUG=chromium:996713, chromium:1072833

Change-Id: Ibb83f9c906fa38f15712ba0f20c67e4929ba3c0e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2192672Reviewed-by: default avatarAustin Eng <enga@chromium.org>
Reviewed-by: default avatarCorentin Wallez <cwallez@chromium.org>
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Cr-Commit-Position: refs/heads/master@{#768579}
parent 1aa0c5b6
......@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/modules/webgpu/dawn_object.h"
#include "gpu/command_buffer/client/webgpu_interface.h"
#include "third_party/blink/renderer/modules/webgpu/gpu_device.h"
namespace blink {
......@@ -29,8 +30,24 @@ const DawnProcTable& DawnObjectBase::GetProcs() const {
return dawn_control_client_->GetProcs();
}
DawnDeviceClientSerializerHolder::DawnDeviceClientSerializerHolder(
scoped_refptr<DawnControlClientHolder> dawn_control_client,
uint64_t device_client_id)
: dawn_control_client_(std::move(dawn_control_client)),
device_client_id_(device_client_id) {}
DawnDeviceClientSerializerHolder::~DawnDeviceClientSerializerHolder() {
if (dawn_control_client_->IsDestroyed()) {
return;
}
dawn_control_client_->GetInterface()->RemoveDevice(device_client_id_);
}
DawnObjectImpl::DawnObjectImpl(GPUDevice* device)
: DawnObjectBase(device->GetDawnControlClient()), device_(device) {}
: DawnObjectBase(device->GetDawnControlClient()),
device_(device),
device_client_serializer_holder_(
device->GetDeviceClientSerializerHolder()) {}
DawnObjectImpl::~DawnObjectImpl() = default;
......
......@@ -42,6 +42,31 @@ class DawnObjectBase {
scoped_refptr<DawnControlClientHolder> dawn_control_client_;
};
// This class allows objects to hold onto a DawnControlClientHolder and a
// device client id. Now one GPUDevice is related to one WebGPUSerializer in
// the client side of WebGPU context. When the GPUDevice and all the other
// WebGPU objects that are created from the GPUDevice are destroyed, this
// object will be destroyed and in the destructor of this object we will
// trigger the clean-ups to the corresponding WebGPUSerailzer and other data
// structures in the GPU process.
class DawnDeviceClientSerializerHolder
: public RefCounted<DawnDeviceClientSerializerHolder> {
public:
DawnDeviceClientSerializerHolder(
scoped_refptr<DawnControlClientHolder> dawn_control_client,
uint64_t device_client_id);
private:
friend class RefCounted<DawnDeviceClientSerializerHolder>;
~DawnDeviceClientSerializerHolder();
scoped_refptr<DawnControlClientHolder> dawn_control_client_;
uint64_t device_client_id_;
};
// TODO(jiawei.shao@intel.com): Remove the redundant reference of
// scoped_refptr<DawnControlClientHolder> inherited from DawnObjectBase as now
// we can access it from device_client_serializer_holder_.
class DawnObjectImpl : public ScriptWrappable, public DawnObjectBase {
public:
DawnObjectImpl(GPUDevice* device);
......@@ -51,6 +76,8 @@ class DawnObjectImpl : public ScriptWrappable, public DawnObjectBase {
protected:
Member<GPUDevice> device_;
scoped_refptr<DawnDeviceClientSerializerHolder>
device_client_serializer_holder_;
};
template <typename Handle>
......@@ -66,17 +93,33 @@ class DawnObject : public DawnObjectImpl {
Handle const handle_;
};
// TODO(jiawei.shao@intel.com): Remove the redundant reference of
// scoped_refptr<DawnControlClientHolder> inherited from DawnObjectBase as now
// we can access it from device_client_serializer_holder_.
template <>
class DawnObject<WGPUDevice> : public DawnObjectBase {
public:
DawnObject(scoped_refptr<DawnControlClientHolder> dawn_control_client,
uint64_t device_client_id,
WGPUDevice handle)
: DawnObjectBase(std::move(dawn_control_client)), handle_(handle) {}
: DawnObjectBase(std::move(dawn_control_client)),
handle_(handle),
device_client_serializer_holder_(
base::MakeRefCounted<DawnDeviceClientSerializerHolder>(
GetDawnControlClient(),
device_client_id)) {}
WGPUDevice GetHandle() const { return handle_; }
const scoped_refptr<DawnDeviceClientSerializerHolder>&
GetDeviceClientSerializerHolder() const {
return device_client_serializer_holder_;
}
private:
WGPUDevice const handle_;
scoped_refptr<DawnDeviceClientSerializerHolder>
device_client_serializer_holder_;
};
} // namespace blink
......
......@@ -40,6 +40,7 @@ GPUDevice::GPUDevice(ExecutionContext* execution_context,
const GPUDeviceDescriptor* descriptor)
: ExecutionContextClient(execution_context),
DawnObject(dawn_control_client,
client_id,
dawn_control_client->GetInterface()->GetDevice(client_id)),
adapter_(adapter),
queue_(MakeGarbageCollected<GPUQueue>(
......@@ -61,7 +62,6 @@ GPUDevice::~GPUDevice() {
}
queue_ = nullptr;
GetProcs().deviceRelease(GetHandle());
GetInterface()->RemoveDevice(client_id_);
}
uint64_t GPUDevice::GetClientID() const {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment