[YevheniiKotyrlo]

Problem

Disposing a surface on one runtime frees its canvas, and finishing a picture recording ends it, under a draw another runtime has in flight on that canvas. Skia objects cross into worklet runtimes by reference, and JsiSkCanvas hands each draw a raw pointer that detach() clears with nothing ordering the two:

SkCanvas *getCanvas() {
  if (_canvas == nullptr) {
    throw std::runtime_error("Attempted to use a canvas whose surface was "
                             "disposed or whose recording has finished.");
  }
  return _canvas;
}

void detach() {
  _canvas = nullptr;
  if (_dispatcher) {
    _dispatcher->release(std::move(_surface));
  }
}

A surface disposed on the thread that made it is destroyed at once (Dispatcher::release()), its SkCanvas with it, and finishRecordingAsPicture() and the Graphite context's finishRecording() end the recording before they detach the canvas. Any app that disposes a surface, or ends a recording, on one runtime while another may be drawing on its canvas meets it: a cleanup on the JS thread while a useFrameCallback worklet draws, for one. On main, the cases below crash the example on Android: SIGSEGV on the JS thread in JsiSkCanvas::drawSvg > SkCanvas::restoreToCount, and on the worklet's thread in finishRecordingAsPicture() > SkRecordFillBounds.

Solution

A draw holds the canvas until it returns, and detach() waits for it. getCanvas() returns a guard that holds the canvas's mutex and is destroyed at the end of the statement, so getCanvas()->drawRect(…) reads as before; drawSvg, Paragraph.paint() and Skottie.render() keep it for the whole call, and readPixels while it reads, letting go of it before its promise is made. detach() takes the same mutex, so it waits for the draw in flight; the surface still goes to its thread's dispatcher. The picture recorder and the Graphite context now detach the canvas before they end the recording. A dispose() on another runtime therefore waits for the one draw call in flight, and the next draw throws the same error as after a dispose() on the same runtime.

No JS runs while a canvas is held, since every method converts its arguments before getCanvas(), so a callback can never detach the canvas its own draw holds. Native code that calls getCanvas() gets the guard: getCanvas()->draw… compiles as before, and .get(), which hands out the raw pointer, compiles only on a guard kept in a variable.

Tests

Two e2e cases, in Surfaces.spec.tsx and Picture.spec.tsx: a worklet runtime (createWorkletRuntime, helpers in apps/example/src/Tests/workletThread.ts) disposes a raster surface it made, or finishes a picture recording, a quarter of the way into a draw on the JS thread that takes a few hundred milliseconds (slowSvg: 262,144 squares, nine levels of nested <use>), the quarter counted from the draw's start against the same draw timed beforehand. Each asserts that the worklet's call started during the draw and was blocked for at least half of what was left of it, and that the JS thread's next draw throws; the recording's case also checks that the picture holds the whole draw. Both crash on main. The lock alone is not enough: with finishRecordingAsPicture() detaching after the finish, the recording's case still crashes.

Verification

cpplint, lint, tsc, test, test-cpp, test-web, build-library and the JVM unit tests pass locally · on a Mi MIX 3 (Android 10), release builds: the two new cases pass, and the other cases of Surfaces, Picture, GraphiteView, Readback, ParagraphPaint, Skottie and SVG pass as on main · the changed headers compile with the iOS and macOS SDKs (Xcode 27.1) with no new warnings · the two new cases pass on an iPhone 12 Pro Max (iOS 27.0.1), Release build. That was on 60170c857; on this head, which merges main 3184116c9, cpplint, lint and tsc pass, the Picture and Surfaces specs pass on the Mi MIX 3 with the two new cases, and the changed headers compile with the iOS and macOS SDKs with main's warnings.

Cost: each call from JS through a canvas takes and releases a mutex nobody else holds: on a Mi MIX 3, getSaveCount 424 → 456 ns, this branch slower by 19–44 ns in each of six pairs of back-to-back launches, where main's own six launches spread 45 ns; a drawRect moves less than main's launches spread (1102 → 1151 ns on a recording canvas against 266 ns, 2188 → 2295 ns on a raster one against 283 ns). The <Canvas> renderer draws in C++ and does not go through it. The evidence comment has the reproduction, the crashes and the measurement.

No existing issue tracks this — searched for dispose canvas worklet, canvas use after free, dispose surface worklet, getCanvas dispose, canvas detached and finishRecordingAsPicture worklet, zero hits.

Related

#4278 made dispose() detach the canvas so that a later draw throws; this covers a draw already in flight on another runtime.

#4353 posts a surface released off the thread that made it to that thread's loop; a dispose() on that thread, the case here, still destroys the surface at once.

Three related races stay outside it: a Graphite context disposed or collected with its recording open never detaches its canvas; getObject() on one runtime copies an object's sk_sp while dispose() on another resets it; and a surface's makeImageSnapshot(), flush() and asImage() take no lock against a draw on its canvas.

[YevheniiKotyrlo]

Reproduction

The two cases this PR adds are the reproduction. With the example app running on an Android device in e2e mode (Metro started with CI=true, which opens its Tests screen), in packages/skia:

E2E=true CI=true yarn test -i "e2e/(Surfaces|Picture).spec" -t "on another runtime"

A worklet runtime made by createWorkletRuntime makes a 1024 × 1024 raster surface and hands it to the JS thread, which draws on its canvas an SVG that takes a few hundred milliseconds (262,144 squares, nine levels of nested <use>); a quarter of the way into the draw, the worklet disposes the surface. In the second case the JS thread records the same SVG into a picture recorder, and a quarter of the way in, the worklet calls finishRecordingAsPicture(); recording the SVG takes less time than rasterizing it.

Before this PR, main at 8e30d2eed — the app dies in both cases, on a Mi MIX 3 (Android 10, release build). The dispose frees the surface, and with it the canvas and its clip stack, under the JS thread's draw; finishRecordingAsPicture() reads the recording while the JS thread is still adding to it.

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), thread mqt_v_js (the JS thread)
#00 __aarch64_ldadd4_acq_rel
#01 SkAAClip::~SkAAClip()
#02 SkRasterClip::~SkRasterClip()
#03 SkBitmapDevice::popClipStack()
#04 SkCanvas::internalRestore()
#05 SkCanvas::restoreToCount(int)
#06 SkSVGRenderContext::~SkSVGRenderContext()
     … SkSVGUse / SkSVGContainer recursion …
#46 SkSVGDOM::render(SkCanvas*) const
#47 RNSkia::JsiSkCanvas::drawSvg(…)

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), the worklet runtime's thread
#00 SkRecord::Record::visit<SkRecords::FillBounds&>(…) const
#01 SkRecordFillBounds(…)
#02 SkPictureRecorder::finishRecordingAsPicture()
#03 RNSkia::JsiSkPictureRecorder::finishRecordingAsPicture()

With this PR — both pass, on the Mi MIX 3 and on an iPhone 12 Pro Max (iOS 27.0.1, Release build). Over five rounds of each on the Mi MIX 3, dispose() started 91–96 ms into a 361–373 ms draw and returned 0–2 ms after the draw ended. finishRecordingAsPicture() started 52–63 ms into a 206–213 ms recording, waited for it to end, and returned 195–225 ms later, the time it takes to build the picture of 262,144 squares (finishing the case's reference recording, with nothing else running, took 226–271 ms), with the whole draw in the picture. Each time, the JS thread's next draw threw "Attempted to use a canvas whose surface was disposed or whose recording has finished."

Device measurement — before / after

main — a call from JS through a canvas costs 424 ns for getSaveCount, 1102 ns for drawRect on a picture recorder's canvas and 2188 ns for drawRect on a 16 × 16 raster surface's canvas.

This PR — each call also takes and releases the canvas's mutex, which no other thread holds in the bench: getSaveCount costs 456 ns, slower than main by 19–44 ns in each of the six pairs of back-to-back launches; the two drawRect calls, 1151 and 2295 ns, move less than main's own six launches spread from fastest to slowest (266 and 283 ns).

ns a call, median of six launches (range)	main	this PR
getSaveCount	424 (398–443)	456 (425–473)
drawRect, picture recorder canvas	1102 (1035–1301)	1151 (1076–1285)
drawRect, raster surface canvas, 16 × 16	2188 (2067–2350)	2295 (1967–2478)

Mi MIX 3 (Android 10), release builds of main 8e30d2eed and of this PR with the same JS; each launch times 20 rounds of 20,000 calls of each of the three calls in turn, and the two builds are launched alternately, six times each. The <Canvas> renderer draws in C++ and does not go through this path. Two earlier runs of six pairs, on earlier commits of this branch whose draw calls run the same code, gave getSaveCount +28 and +47 ns, the branch slower in all twelve pairs.

The bench screen, each launch's readings and the script that reads them back into the table, the timing probe behind the numbers above and the two crashes' backtraces are in my evidence repository, not in the PR: https://github.com/YevheniiKotyrlo/react-native-skia-evidence/tree/main/pr-4356.