[emdawnwebgpu] Fix two refcounting bugs, and fix for 64-bit flags
- Ref::~Ref was calling the virtual method Release(), which for
RefCountedWithExternalCount releases an external ref (but Ref<> is
supposed to only be an internal ref). I stumbled on this; it did not
actually cause any crashes, but the external refcount value was wrong.
- There was an atomic fetch_add() inside an assert(), which was skipped
in release builds.
- Switching bitflags to 64-bit broke the Wasm->JS calling convention (in
a very hard to find way as usual), so did the TODO that was there to
fix it.
- Add some runtimeKeepalivePush/Pop calls for the DeviceLost callbacks.
This seems to be the only instance of `.then()` that didn't have it.
(I didn't actually see a crash here, I just noticed it. Note, this is
an odd case because it may never be called, but I think this behavior
is correct. In the future we might be able to destroy() the device
when all of its internal and external refs reach 0? Then the
DeviceLost callbacks should get called and the runtime can shut down.)
Discovered with and debugged/tested against this project:
https://github.com/kainino0x/webgpu-cross-platform-demo/tree/dawnwasm
Fixed: 347732150
Bug: none
Change-Id: I6cb3775b3ea7f63772cb79564e732df77c8aab68
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/205874
Auto-Submit: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/generator/dawn_json_generator.py b/generator/dawn_json_generator.py
index 4932e42..8ffa894 100644
--- a/generator/dawn_json_generator.py
+++ b/generator/dawn_json_generator.py
@@ -965,8 +965,7 @@
if x.category == 'enum':
return 'i'
elif x.category == 'bitmask':
- # TODO(crbug.com/347732150): Change to 'j' when bitmasks are 64-bit
- return 'i'
+ return 'j'
elif x.category in ['object', 'function pointer']:
return 'p'
elif x.category == 'native':
diff --git a/third_party/emdawnwebgpu/library_webgpu.js b/third_party/emdawnwebgpu/library_webgpu.js
index f7a1594..b467d15 100644
--- a/third_party/emdawnwebgpu/library_webgpu.js
+++ b/third_party/emdawnwebgpu/library_webgpu.js
@@ -703,7 +703,9 @@
// Set up device lost promise resolution.
if (hasDeviceLostFutureId) {
+ {{{ runtimeKeepalivePush() }}}
WebGPU._futureInsert(deviceLostFutureIdL, deviceLostFutureIdH, device.lost.then((info) => {
+ {{{ runtimeKeepalivePop() }}}
// Unset the uncaptured error handler.
device.onuncapturederror = (ev) => {};
withStackSave(() => {
diff --git a/third_party/emdawnwebgpu/webgpu.cpp b/third_party/emdawnwebgpu/webgpu.cpp
index 1252d04..9933fe7 100644
--- a/third_party/emdawnwebgpu/webgpu.cpp
+++ b/third_party/emdawnwebgpu/webgpu.cpp
@@ -84,7 +84,9 @@
static constexpr bool HasExternalRefCount = false;
void AddRef() {
- assert(mRefCount.fetch_add(1u, std::memory_order_relaxed) >= 1);
+ [[maybe_unused]] uint64_t oldRefCount =
+ mRefCount.fetch_add(1u, std::memory_order_relaxed);
+ assert(oldRefCount >= 1);
}
void Release() {
@@ -135,11 +137,7 @@
"Cannot make a Ref<T> when T is not a Refcounted type.");
Ref() : mValue(nullptr) {}
- ~Ref() {
- if (mValue) {
- mValue->Release();
- }
- }
+ ~Ref() { Release(mValue); }
// Constructors from nullptr.
// NOLINTNEXTLINE(runtime/explicit)
