diff --git a/cc/paint/paint_op.cc b/cc/paint/paint_op.cc index 968a50f52e8ed..dc3eb8e59134a 100644 --- a/cc/paint/paint_op.cc +++ b/cc/paint/paint_op.cc @@ -2647,7 +2647,48 @@ DrawVerticesOp::DrawVerticesOp( uvs(std::move(uvs)), indices(std::move(indices)) {} -DrawVerticesOp::~DrawVerticesOp() = default; +DrawVerticesOp::~DrawVerticesOp() { + // --------------------------------------------------------------------------- + // POC race-window widener (compositor-thread side). + // + // Semantically identical to what the implicit `scoped_refptr` member dtors + // would do via base::RefCountedBase::Release(): a non-atomic `--ref_count_` + // followed by a SEPARATE `ref_count_ == 0` load. The only difference is a + // busy-spin between the two, widening the existing few-cycle window so the + // cross-thread race against blink::Mesh2DBuffer (main-thread AddRef/Release + // on the SAME cc::RefCountedBuffer) becomes reliably observable under ASAN. + // --------------------------------------------------------------------------- + auto widened_release = [](auto& sp) { + auto* p = sp.release(); + if (!p) { + return; + } + auto& rc = *reinterpret_cast( + const_cast( + static_cast(p))); + // base::RefCountedBase::ReleaseImpl(): non-atomic `--ref_count_`, i.e. + // load + dec + store, with the load/store gap widened so concurrent + // main-thread AddRef()s (drawMesh -> GetBuffer()) are lost. + uint32_t old = rc; + for (int i = 0; i < 100000; ++i) { + asm volatile("" ::: "memory"); + } + rc = old - 1; + // base::RefCountedBase::Release(): SEPARATE zero-check. + for (int i = 0; i < 100000; ++i) { + asm volatile("" ::: "memory"); + } + if (rc == 0) { + // ~RefCountedBuffer is private; route the delete through the public + // Release() by restoring the count we just took. + ++rc; + p->Release(); + } + }; + widened_release(vertices); + widened_release(uvs); + widened_release(indices); +} DrawSkottieOp::DrawSkottieOp(scoped_refptr skottie, SkRect dst, diff --git a/cc/paint/refcounted_buffer.h b/cc/paint/refcounted_buffer.h index a2c950090abc3..002efa1dad101 100644 --- a/cc/paint/refcounted_buffer.h +++ b/cc/paint/refcounted_buffer.h @@ -31,6 +31,37 @@ class CC_PAINT_EXPORT RefCountedBuffer return buffer_ == other.buffer_; } + // --------------------------------------------------------------------------- + // POC race-window widener. + // + // This shadows base::RefCounted::Release() with a SEMANTICALLY IDENTICAL + // implementation: a non-atomic `--ref_count_` followed by a SEPARATE + // `ref_count_ == 0` load (exactly what RefCountedBase::Release() does in + // release builds, see base/memory/ref_counted.h:72-90 + ref_counted.cc:59). + // The only difference is a busy-spin between the decrement and the load, + // widening the existing 1-3-cycle race window so the cross-thread access + // (compositor-thread ~DrawVerticesOp vs main-thread drawMesh()/Oilpan-sweep) + // becomes reliably observable under ASAN. Removing the spin restores the + // exact upstream timing without changing reachability of the bug. + void Release() const { + // base::subtle::RefCountedBase has no vtable and `ref_count_` is its first + // (and, in non-DCHECK builds, only) data member -> offset 0. + auto& ref_count = *reinterpret_cast( + const_cast( + static_cast(this))); + // 1) base::RefCountedBase::ReleaseImpl(): non-atomic decrement. + --ref_count; + // 2) Widen the pre-existing window between the decrement and the re-load. + for (int i = 0; i < 2000; ++i) { + asm volatile("" ::: "memory"); + } + // 3) base::RefCountedBase::Release(): SEPARATE load + zero check. + if (ref_count == 0) { + delete this; + } + } + // --------------------------------------------------------------------------- + private: friend class base::RefCounted>; ~RefCountedBuffer() = default;