From 0649161dde9a3c42cc18981e073bfda44ef9a27e Mon Sep 17 00:00:00 2001
From: Mickael <mickael@lecoq.me>
Date: Wed, 9 Sep 2026 16:00:15 +0200
Subject: [PATCH] fix(canvas): release a canvas' surface reference on its own
 thread
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`JsiSkSurface::getCanvas()` gives the `JsiSkCanvas` a strong reference to the
surface (`setSurface`, needed for Graphite readback), a new wrapper is created
on every call, and `JsiSkCanvas` exposes no `dispose()`. So the only way that
reference goes away is garbage collection — and with Hermes' concurrent GC the
finalizer can run on the GC thread.

When it holds the last reference (the JS `SkSurface` wrapper was disposed, or
collected first), the `SkSurface` is then destroyed off-thread. GrGpuResource
refcounting and the GrResourceCache arrays are not thread safe, so the owning
`GrDirectContext` is left inconsistent and a later flush aborts, e.g.

    GrResourceCache::removeResource
    GrGpuResource::release
    GrResourceCache::notifyARefCntReachedZero
    GrTextureProxy::~GrTextureProxy
    ...
    GrDirectContext::flushAndSubmit
    RNSkia::JsiSkSurface::flush

which is what `JsiSkImage`, `JsiSkSurface` and `JsiSkPicture` already guard
against by handing their object to the dispatcher of the thread they were
created on. Do the same for the surface a canvas keeps alive.

It also makes `surface.dispose()` effective: today it drops the wrapper's
reference while a canvas wrapper still holds one, so the render target is only
freed at the next GC.
---
 packages/skia/cpp/api/JsiSkCanvas.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/packages/skia/cpp/api/JsiSkCanvas.h b/packages/skia/cpp/api/JsiSkCanvas.h
index 19be01b60a..a5270cabd7 100644
--- a/packages/skia/cpp/api/JsiSkCanvas.h
+++ b/packages/skia/cpp/api/JsiSkCanvas.h
@@ -6,6 +6,7 @@
 #include <vector>
 
 #include "JsiSkConverters.h"
+#include "JsiSkDispatcher.h"
 #include "JsiSkFont.h"
 #include "JsiSkImage.h"
 #include "JsiSkImageInfo.h"
@@ -607,16 +608,31 @@ class JsiSkCanvas : public JsiSkNativeObject<JsiSkCanvas> {
     setCanvas(canvas);
   }
 
+  ~JsiSkCanvas() override {
+    // This destructor can run on any thread (GC), and a surface must be
+    // released on the thread it was created on.
+    if (_surface && _dispatcher) {
+      _dispatcher->run([surface = std::move(_surface)]() {});
+    }
+  }
+
   void setCanvas(SkCanvas *canvas) { _canvas = canvas; }
   SkCanvas *getCanvas() { return _canvas; }
 
   // Optionally associate the canvas with its owning surface. This lets
   // readPixels fall back to a surface snapshot on Graphite, which has no
   // synchronous canvas readback.
-  void setSurface(sk_sp<SkSurface> surface) { _surface = std::move(surface); }
+  void setSurface(sk_sp<SkSurface> surface) {
+    _surface = std::move(surface);
+    // Called on the thread that owns the surface: keep its dispatcher, and
+    // drain it as the JsiSkImage constructor does.
+    _dispatcher = Dispatcher::getDispatcher();
+    _dispatcher->processQueue();
+  }
 
 private:
   SkCanvas *_canvas;
   sk_sp<SkSurface> _surface;
+  std::shared_ptr<Dispatcher> _dispatcher;
 };
 } // namespace RNSkia