From 89beee65147ef47d9c35e163087b01643825d7bb Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Mon, 29 Jun 2026 12:13:34 +1000 Subject: [PATCH] ifcviewer: detect web pool-grow OOM via provisional sub-buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On web we skip the desktop error-scope spin-wait (it blocks the JS event loop and hangs the page). The old web addSubBuffer then judged success by `buf != nullptr` — but Dawn-web returns a NON-NULL error buffer on OOM, so the pool committed an invalid sub-buffer, alloc handed out slices in it, and every chunk_bind_group built against it failed ("BindGroup is invalid" spam + a wgpuQueueSubmit panic). Loading a model larger than the browser's WebGPU budget triggered exactly this. Add the grown sub-buffer as *provisional* (alloc and the capacity/free tallies skip it) and validate it through a non-blocking AllowSpontaneous PopErrorScope. resolveProvisionalGrowth() clears the flag when it's good, or drops the sub-buffer and latches growth_disabled_ on a real OOM — at which point the streaming evictor bounds the working set to what fits instead of cascading. Only one provisional grow is in flight at a time (growth_pending_). Desktop keeps its synchronous halve-retry path unchanged. All 100 unit tests still pass. Co-Authored-By: Claude Opus 4.8 --- src/ifcviewer/BufferPool.cpp | 105 ++++++++++++++++++++++++++++++----- src/ifcviewer/BufferPool.h | 18 ++++++ 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/src/ifcviewer/BufferPool.cpp b/src/ifcviewer/BufferPool.cpp index fa690be484..2dfde93b3e 100644 --- a/src/ifcviewer/BufferPool.cpp +++ b/src/ifcviewer/BufferPool.cpp @@ -51,6 +51,7 @@ void BufferPool::destroy() { per_sub_buffer_capacity_ = 0; last_growth_size_ = 0; growth_disabled_ = false; + growth_pending_ = false; label_prefix_.clear(); } @@ -69,6 +70,57 @@ bool BufferPool::addSubBuffer() { : per_sub_buffer_capacity_; if (try_size < MIN_SUB_BUFFER_BYTES) try_size = MIN_SUB_BUFFER_BYTES; +#if defined(__EMSCRIPTEN__) + // Web can't synchronously learn whether createBuffer OOM'd: the + // desktop spin-wait that drains PopErrorScope would block the JS + // event loop so the resolving microtask never runs (page hangs), and + // Dawn returns a NON-NULL error buffer on OOM — so a plain + // `buf != nullptr` check silently accepts an invalid buffer, and + // every bind group built against it then fails ("BindGroup is + // invalid" spam). Instead add the sub-buffer as *provisional* (alloc + // skips it), then validate it through a non-blocking async error + // scope. resolveProvisionalGrowth() clears the flag once it's known + // good, or drops it and latches growth_disabled_ on a real OOM. Only + // one provisional grow is ever in flight (growth_pending_), so a hung + // validation can't spawn a pile of sub-buffers. + if (growth_pending_) return false; + + char label[128]; + std::snprintf(label, sizeof(label), "%s.sub%zu", + label_prefix_.c_str(), sub_pools_.size()); + WGPUBufferDescriptor desc = {}; + desc.usage = usage_; + desc.size = try_size; + desc.label.data = label; + desc.label.length = std::strlen(label); + + wgpuDevicePushErrorScope(device_, WGPUErrorFilter_OutOfMemory); + WGPUBuffer buf = wgpuDeviceCreateBuffer(device_, &desc); + + SubPool sp; + sp.buffer = buf; + sp.capacity = try_size; + sp.used = 0; + sp.provisional = true; + sp.free_ranges.push_back({0, try_size}); + sub_pools_.push_back(std::move(sp)); + last_growth_size_ = try_size; + growth_pending_ = true; + + WGPUPopErrorScopeCallbackInfo pcb = {}; + pcb.mode = WGPUCallbackMode_AllowSpontaneous; + pcb.callback = [](WGPUPopErrorScopeStatus, WGPUErrorType type, + WGPUStringView, void* ud1, void* /*ud2*/) { + static_cast(ud1)->resolveProvisionalGrowth( + type != WGPUErrorType_NoError); + }; + pcb.userdata1 = this; + wgpuDevicePopErrorScope(device_, pcb); + + // No usable space yet: the provisional sub-buffer isn't handed out + // until validated. alloc fails this frame and retries on a later one. + return false; +#else while (try_size >= MIN_SUB_BUFFER_BYTES) { char label[128]; std::snprintf(label, sizeof(label), "%s.sub%zu", @@ -80,17 +132,6 @@ bool BufferPool::addSubBuffer() { desc.label.data = label; desc.label.length = std::strlen(label); -#if defined(__EMSCRIPTEN__) - // Web: skip the error-scope dance. PopErrorScope on Dawn-web - // resolves via JS microtask, and the spin-wait below (the - // desktop path) would block the JS event loop indefinitely — - // microtask never gets to fire, page hangs. We just trust the - // browser: if createBuffer succeeded the buffer is usable, - // and if it failed `buf` is null and we halve-retry as - // before. A real OOM still surfaces as `buf == nullptr` here. - WGPUBuffer buf = wgpuDeviceCreateBuffer(device_, &desc); - const bool ok = (buf != nullptr); -#else // wgpu-native classifies "Not enough memory left" as Validation, // not OutOfMemory. Nested scopes: OOM inner, Validation outer. wgpuDevicePushErrorScope(device_, WGPUErrorFilter_Validation); @@ -116,7 +157,6 @@ bool BufferPool::addSubBuffer() { pop(oom_pop); pop(validation_pop); const bool ok = buf && !oom_pop.error && !validation_pop.error; -#endif if (ok) { SubPool sp; @@ -145,8 +185,38 @@ bool BufferPool::addSubBuffer() { sub_pools_.size()); growth_disabled_ = true; return false; +#endif // __EMSCRIPTEN__ } +#if defined(__EMSCRIPTEN__) +void BufferPool::resolveProvisionalGrowth(bool failed) { + growth_pending_ = false; + // The provisional sub-pool is the most recently added; locate it from + // the back (growth_pending_ guaranteed no others were appended). + for (size_t i = sub_pools_.size(); i-- > 0; ) { + if (!sub_pools_[i].provisional) continue; + if (failed) { + if (sub_pools_[i].buffer) wgpuBufferRelease(sub_pools_[i].buffer); + sub_pools_.erase(sub_pools_.begin() + i); + growth_disabled_ = true; + std::fprintf(stderr, + "[wgpu pool] sub-buffer grow OOM'd; pool capped at %llu MB " + "across %zu sub-buffer(s) — growth disabled\n", + (unsigned long long)(total_capacity_bytes() / (1024 * 1024)), + sub_pools_.size()); + } else { + sub_pools_[i].provisional = false; + std::fprintf(stderr, + "[wgpu pool] added sub-buffer %zu (%llu MB); pool total now %llu MB\n", + i, + (unsigned long long)(sub_pools_[i].capacity / (1024 * 1024)), + (unsigned long long)(total_capacity_bytes() / (1024 * 1024))); + } + return; + } +} +#endif // __EMSCRIPTEN__ + BufferPool::Slice BufferPool::alloc(uint64_t size, uint64_t align) { Slice out; if (size == 0 || align == 0) return out; @@ -156,6 +226,9 @@ BufferPool::Slice BufferPool::alloc(uint64_t size, uint64_t align) { for (int attempt = 0; attempt < 2; ++attempt) { for (size_t sp_idx = 0; sp_idx < sub_pools_.size(); ++sp_idx) { SubPool& sp = sub_pools_[sp_idx]; + // Web: never allocate out of a sub-buffer still awaiting OOM + // validation — its handle may be a Dawn error buffer. + if (sp.provisional) continue; for (size_t i = 0; i < sp.free_ranges.size(); ++i) { const FreeRange& r = sp.free_ranges[i]; const uint64_t aligned = (r.offset + (align - 1)) & ~(align - 1); @@ -219,19 +292,23 @@ void BufferPool::free(const Slice& s) { uint64_t BufferPool::total_capacity_bytes() const { uint64_t s = 0; - for (const auto& sp : sub_pools_) s += sp.capacity; + // Skip provisional sub-pools (web, awaiting OOM validation) — their + // capacity isn't usable yet, so counting it would mislead the + // evictor's "is there room?" heuristics. + for (const auto& sp : sub_pools_) if (!sp.provisional) s += sp.capacity; return s; } uint64_t BufferPool::total_used_bytes() const { uint64_t s = 0; - for (const auto& sp : sub_pools_) s += sp.used; + for (const auto& sp : sub_pools_) if (!sp.provisional) s += sp.used; return s; } uint64_t BufferPool::largest_free_run_bytes() const { uint64_t m = 0; for (const auto& sp : sub_pools_) { + if (sp.provisional) continue; for (const auto& r : sp.free_ranges) { if (r.size > m) m = r.size; } diff --git a/src/ifcviewer/BufferPool.h b/src/ifcviewer/BufferPool.h index cbfeef0ea2..fdbcf54b4d 100644 --- a/src/ifcviewer/BufferPool.h +++ b/src/ifcviewer/BufferPool.h @@ -132,6 +132,11 @@ private: uint64_t capacity = 0; uint64_t used = 0; std::vector free_ranges; + // Web-only: true between addSubBuffer creating the buffer and the + // async error scope confirming it didn't OOM. alloc() and the + // capacity/free tallies skip provisional sub-pools so an + // unvalidated (possibly invalid) buffer is never handed out. + bool provisional = false; }; // Append a new sub-buffer to the pool. Starts at last_growth_size_ @@ -148,6 +153,15 @@ private: // refused, at which point growth_disabled_ latches. bool addSubBuffer(); +#if defined(__EMSCRIPTEN__) + // Web-only async-growth resolver. Called from the AllowSpontaneous + // PopErrorScope callback addSubBuffer arms: `failed` true means the + // provisional sub-buffer OOM'd → drop it and latch growth_disabled_; + // false means it's good → clear its provisional flag so alloc can use + // it. Clears growth_pending_ either way. + void resolveProvisionalGrowth(bool failed); +#endif + std::vector sub_pools_; WGPUInstance instance_ = nullptr; @@ -162,6 +176,10 @@ private: // subsequent grow is wasted work. uint64_t last_growth_size_ = 0; bool growth_disabled_ = false; + // Web-only: a provisional sub-buffer is awaiting async OOM validation. + // Blocks a second concurrent grow so a stalled validation can't spawn + // a pile of sub-buffers. + bool growth_pending_ = false; std::string label_prefix_; };