Shopify / Shopify/react-native-skia

Android: every non-opaque Canvas leaks one android.view.Surface

Open
#4,045 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
8.6k
Forks
647
Avg merge
1d 17h
Merged PRs (30d)
35

Description

Description

On Android, every non-opaque <Canvas> leaks one android.view.Surface.

RNSkOpenGLCanvasProvider::surfaceAvailable constructs a Surface to wrap the SurfaceTexture, passes it to ANativeWindow_fromSurface, then drops it with DeleteLocalRef — which only releases the JNI local reference. It never calls Surface.release(), so the underlying buffer producer is left to the finalizer.

Android's CloseGuard reports it at runtime, one line per Canvas:

W System: A resource failed to call Surface.release.
Affected code

packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp, in the if (!opaque) branch of surfaceAvailable:

auto jSurface =
    env->NewObject(surfaceClass, surfaceConstructor, jSurfaceTexture);
window = ANativeWindow_fromSurface(env, jSurface);
...
env->DeleteLocalRef(jSurface);   // no jSurface.release()

surfaceDestroyed() releases _jSurfaceTexture via DeleteGlobalRef, and OpenGLWindowContext.h releases the ANativeWindow, but nothing ever owns the intermediate Surface.

The opaque branch is unaffected — it passes the SurfaceTexture directly with no wrapper object.

Contrast with the WebGPU path

packages/skia/android/cpp/jni/JniWebGPUView.cpp documents this exact ownership rule and honours it:

// ANativeWindow_fromSurface acquires a reference; SurfaceInfo releases it
// (via the releaser below) once it is done with the window.

with a matching ANativeWindow_release(...). The OpenGL path just misses the corresponding release of the Java wrapper.

Version

RNSkOpenGLCanvasProvider.cpp is byte-identical across 2.9.1, 2.10.2 and 2.11.2 (verified by diff), and the same code is on main today — so every release in that range is affected.

Steps to reproduce
  1. Render any <Canvas> on Android without opaque (the default).
  2. adb logcat | grep "Surface.release".
  3. One W System: A resource failed to call Surface.release. per Canvas.
Observed

An app with 6 <Canvas> components (a tab bar of Skia icons):

canvases created Surface.release warnings
2.9.1 as shipped 6 6
with the fix below 6 0

Reproduced on:

  • Samsung SM-M366B, Android API 36, Mali GPU — debug and minified release build
  • Android emulator API 37

Counts stayed flat across 20+ navigations, so it is one leaked Surface per Canvas creation rather than an unbounded leak — but it is a leak, and it scales with the number of canvases an app mounts.

Suggested fix

Call Surface.release() once ANativeWindow_fromSurface has taken its own reference on the producer:

jmethodID surfaceRelease = env->GetMethodID(surfaceClass, "release", "()V");
if (surfaceRelease != nullptr) {
  env->CallVoidMethod(jSurface, surfaceRelease);
  if (env->ExceptionCheck()) {
    env->ExceptionClear();
  }
}

I have opened a PR with this change.

Verified after the fix, on the same devices: 0 Surface.release warnings, and no BufferQueue has been abandoned, EGL_BAD_SURFACE, EGL_BAD_NATIVE_WINDOW, SIGSEGV or ANR across 20 navigations plus background/foreground cycles. Rendering is unchanged.

Note on a related, separate issue

surfaceSizeChanged re-enters surfaceAvailable when _surfaceHolder == nullptr, and surfaceAvailable assigns _jSurfaceTexture = env->NewGlobalRef(...) without DeleteGlobalRef on any previous value — a JNI global-reference leak on the resize/rotation path. I have not touched it in the PR, since it is a distinct defect and I could not exercise that path (the app I reproduced on is portrait-locked). Flagging it in case it is worth a separate fix.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp, specifically the non-opaque branch of surfaceAvailable, and compare its ownership handling with packages/skia/android/cpp/jni/JniWebGPUView.cpp. Reproduce with adb logcat while mounting non-opaque Canvas components; done means the warnings disappear without rendering regressions, though the issue states that a pull request is already open.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, cpp
Domain
mobile
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.