Skip to content

Commit 06dbef1

Browse files
authored
Refactor WebGPU surface lifecycle to support latched attach (#426)
The SurfaceRegistry kept the sync getContext("webgpu") contract by falling back to an offscreen texture when the native surface is not available yet, but the reconciliation points were racy and crash-prone. This reworks the lifecycle around explicit invariants: - Registry entries now live exactly as long as their JS Canvas (contextIds are never reused). Native view teardown (MetalView dealloc / onDropViewInstance) retires the entry when a surface is attached; RNWebGPU.destroyContext (Canvas unmount cleanup) covers entries that never had one, which keeps StrictMode's simulated unmount from orphaning a live surface. Surface destruction alone (backgrounding, TextureView teardown) only detaches — fixing the orphaned-context black screen on the transparent/TextureView path and the SurfaceView/TextureView asymmetry. - Surface attaches are latched by the UI thread and adopted at frame boundaries on the rendering thread (start of getCurrentTexture / end of present), so a surface is never swapped mid-frame and Dawn surface thread-affinity is preserved. A CallInvoker flush covers contexts that are not actively rendering. Detaches stay immediate (the platform is destroying the surface) and drop the in-flight frame at present(). - present() only presents frames whose texture was actually acquired from the attached surface, eliminating present-without-acquire validation errors around transitions. - The offscreen->onscreen blit now checks SurfaceTexture.status, clamps the copy extent to the shared region, presents from the rendering thread, and no longer permanently widens the configured usage with CopyDst. - getCurrentTexture checks SurfaceTexture.status (reconfigure + retry once, then offscreen fallback), throws a catchable JS error when called before configure() instead of dereferencing a null device, and clamps zero-sized canvases to 1x1. unconfigure() is now implemented. - viewFormats are deep-copied into SurfaceInfo; the stored configuration previously kept a pointer into Convertor-owned memory that died with the configure() call. - ANativeWindow references are now released (acquire in onSurfaceCreate was never paired), the registry is cleared on RNWebGPUManager teardown so dev reloads cannot alias new canvases onto stale entries, and every registry lookup on the native side is null-checked. Removes the unused WebGPUBaseView (its JNI bindings never existed) and the dead onSurfaceDestroy path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XLLf4VPzGb642Zp8EyNgpr * fix(surface): close the races found by adversarial review of the attach model Three defects confirmed by review of the latched-attach rework: - Double-present race: between applyPendingAttach's blit submit and its deferred Present(), a frame could start and complete on another runtime; the !_frameInFlight guard missed frames that finished inside the gap and Present() fired with no acquired texture. A _frameEpoch counter (bumped on every acquire, present, configure/reconfigure/unconfigure, adoption, and detach) is now revalidated before the deferred present. - destroyContext TOCTOU: hasNativeSurface() (SurfaceInfo mutex) followed by removeSurfaceInfo (registry mutex) was not atomic against the UI thread's find-or-create + attach, so a StrictMode cleanup racing a native mount could orphan a just-attached surface. SurfaceRegistry now provides attachSurface (find-or-create + attach) and removeSurfaceInfoIfDetached (lookup + check + erase), both atomic under the registry lock; lock order is registry -> SurfaceInfo on every path. - iOS CAMetalLayer use-after-free: the latched attach stored an unretained layer pointer; on dev reload the registry is cleared before MetalView dealloc runs, so dealloc could not cancel the pending attach and the queued flush later touched a freed layer. The layer is now CFBridgingRetain'd for the lifetime of the attach and released on the main queue via the same releaser mechanism as Android's ANativeWindow ref.
1 parent e4f7f7a commit 06dbef1

18 files changed

Lines changed: 673 additions & 255 deletions

packages/webgpu/android/cpp/cpp-adapter.cpp

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ extern "C" JNIEXPORT void JNICALL Java_com_webgpu_WebGPUModule_initializeNative(
2525
auto jsCallInvoker{
2626
facebook::jni::alias_ref<facebook::react::CallInvokerHolder::javaobject>{
2727
reinterpret_cast<facebook::react::CallInvokerHolder::javaobject>(
28-
jsCallInvokerHolder)} -> cthis()
29-
->getCallInvoker()};
28+
jsCallInvokerHolder)} -> cthis()->getCallInvoker()};
3029
auto platformContext =
3130
std::make_shared<rnwgpu::AndroidPlatformContext>(globalBlobModule);
3231
manager = std::make_shared<rnwgpu::RNWebGPUManager>(runtime, jsCallInvoker,
@@ -37,35 +36,58 @@ extern "C" JNIEXPORT void JNICALL Java_com_webgpu_WebGPUView_onSurfaceChanged(
3736
JNIEnv *env, jobject thiz, jobject surface, jint contextId, jfloat width,
3837
jfloat height) {
3938
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
40-
registry.getSurfaceInfo(contextId)->resize(static_cast<int>(width),
41-
static_cast<int>(height));
39+
if (auto info = registry.getSurfaceInfo(contextId)) {
40+
info->resize(static_cast<int>(width), static_cast<int>(height));
41+
}
4242
}
4343

4444
extern "C" JNIEXPORT void JNICALL Java_com_webgpu_WebGPUView_onSurfaceCreate(
4545
JNIEnv *env, jobject thiz, jobject jSurface, jint contextId, jfloat width,
4646
jfloat height) {
47+
if (manager == nullptr) {
48+
return;
49+
}
50+
// ANativeWindow_fromSurface acquires a reference; SurfaceInfo releases it
51+
// (via the releaser below) once it is done with the window.
4752
auto window = ANativeWindow_fromSurface(env, jSurface);
48-
// ANativeWindow_acquire(window);
53+
if (window == nullptr) {
54+
return;
55+
}
4956
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
5057
auto gpu = manager->_gpu;
5158
auto surface = manager->_platformContext->makeSurface(
5259
gpu, window, static_cast<int>(width), static_cast<int>(height));
53-
registry
54-
.getSurfaceInfoOrCreate(contextId, gpu, static_cast<int>(width),
55-
static_cast<int>(height))
56-
->switchToOnscreen(window, surface);
60+
// Find-or-create + attach runs atomically under the registry lock so a
61+
// concurrent destroyContext cannot orphan this surface.
62+
auto info = registry.attachSurface(
63+
contextId, gpu, static_cast<int>(width), static_cast<int>(height), window,
64+
surface, [](void *nativeSurface) {
65+
ANativeWindow_release(static_cast<ANativeWindow *>(nativeSurface));
66+
});
67+
// The attach is adopted at the next frame boundary by the rendering thread;
68+
// schedule a flush so contexts that are not currently rendering still pick
69+
// it up (and present their last offscreen frame).
70+
manager->flushPendingSurfaceTransition(info);
5771
}
5872

5973
extern "C" JNIEXPORT void JNICALL
6074
Java_com_webgpu_WebGPUView_switchToOffscreenSurface(JNIEnv *env, jobject thiz,
6175
jint contextId) {
6276
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
63-
auto nativeSurface = registry.getSurfaceInfo(contextId)->switchToOffscreen();
64-
// ANativeWindow_release(reinterpret_cast<ANativeWindow *>(nativeSurface));
77+
if (auto info = registry.getSurfaceInfo(contextId)) {
78+
info->switchToOffscreen();
79+
}
6580
}
6681

67-
extern "C" JNIEXPORT void JNICALL Java_com_webgpu_WebGPUView_onSurfaceDestroy(
82+
extern "C" JNIEXPORT void JNICALL Java_com_webgpu_WebGPUView_onViewDestroyed(
6883
JNIEnv *env, jobject thiz, jint contextId) {
84+
// The view dies with its Canvas (contextIds are never reused), so view
85+
// teardown retires the registry entry. The JS-side cleanup
86+
// (RNWebGPU.destroyContext) only handles entries that never had a native
87+
// surface; see RNWebGPU::destroyContext for the ownership split.
6988
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
89+
if (auto info = registry.getSurfaceInfo(contextId)) {
90+
info->detachSurface();
91+
}
7092
registry.removeSurfaceInfo(contextId);
71-
}
93+
}

packages/webgpu/android/src/main/java/com/webgpu/WebGPUAHBView.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,9 @@ private void cleanupImageReader() {
254254
protected void onDetachedFromWindow() {
255255
super.onDetachedFromWindow();
256256

257-
// Notify WebGPU that surface is being destroyed
257+
// Detach: rendering falls back to the offscreen texture until reattach.
258258
if (mSurfaceCreated) {
259-
mApi.surfaceDestroyed();
259+
mApi.surfaceOffscreen();
260260
mSurfaceCreated = false;
261261
}
262262

packages/webgpu/android/src/main/java/com/webgpu/WebGPUAPI.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44

55
import com.facebook.proguard.annotations.DoNotStrip;
66

7+
/**
8+
* Surface lifecycle events a WebGPU child view reports. The registry entry
9+
* itself is owned by the JS Canvas component (created lazily, removed via
10+
* RNWebGPU.destroyContext on unmount); views only attach and detach surfaces.
11+
* A detached context keeps rendering into an offscreen texture whose content
12+
* is blitted onto the next attached surface.
13+
*/
714
public interface WebGPUAPI {
815

916
void surfaceCreated(
@@ -14,7 +21,5 @@ void surfaceChanged(
1421
Surface surface
1522
);
1623

17-
void surfaceDestroyed();
18-
1924
void surfaceOffscreen();
2025
}

packages/webgpu/android/src/main/java/com/webgpu/WebGPUBaseView.java

Lines changed: 0 additions & 61 deletions
This file was deleted.

packages/webgpu/android/src/main/java/com/webgpu/WebGPUSurfaceView.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ public WebGPUSurfaceView(Context context, WebGPUAPI api) {
2121
@Override
2222
protected void onDetachedFromWindow() {
2323
super.onDetachedFromWindow();
24-
mApi.surfaceDestroyed();
24+
// surfaceDestroyed() normally fires during detach as well; going offscreen
25+
// is idempotent, so this is just a safety net for paths where it does not.
26+
mApi.surfaceOffscreen();
2527
}
2628

2729
@Override

packages/webgpu/android/src/main/java/com/webgpu/WebGPUTextureView.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,11 @@
77
import android.view.TextureView;
88
import androidx.annotation.NonNull;
99

10-
import org.w3c.dom.Text;
11-
1210
@SuppressLint("ViewConstructor")
1311
public class WebGPUTextureView extends TextureView implements TextureView.SurfaceTextureListener {
1412

1513
WebGPUAPI mApi;
14+
private Surface mSurface;
1615

1716
public WebGPUTextureView(Context context, WebGPUAPI api) {
1817
super(context);
@@ -23,19 +22,24 @@ public WebGPUTextureView(Context context, WebGPUAPI api) {
2322

2423
@Override
2524
public void onSurfaceTextureAvailable(@NonNull SurfaceTexture surfaceTexture, int width, int height) {
26-
Surface surface = new Surface(surfaceTexture);
27-
mApi.surfaceCreated(surface);
25+
mSurface = new Surface(surfaceTexture);
26+
mApi.surfaceCreated(mSurface);
2827
}
2928

3029
@Override
3130
public void onSurfaceTextureSizeChanged(@NonNull SurfaceTexture surfaceTexture, int width, int height) {
32-
Surface surface = new Surface(surfaceTexture);
33-
mApi.surfaceChanged(surface);
31+
mApi.surfaceChanged(mSurface);
3432
}
3533

3634
@Override
3735
public boolean onSurfaceTextureDestroyed(@NonNull SurfaceTexture surfaceTexture) {
38-
mApi.surfaceDestroyed();
36+
// Detach first (synchronous through JNI) so the native side has dropped
37+
// its window reference before we release ours.
38+
mApi.surfaceOffscreen();
39+
if (mSurface != null) {
40+
mSurface.release();
41+
mSurface = null;
42+
}
3943
return true;
4044
}
4145

packages/webgpu/android/src/main/java/com/webgpu/WebGPUView.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,19 @@ public void surfaceChanged(Surface surface) {
7373
onSurfaceChanged(surface, mContextId, width, height);
7474
}
7575

76-
@Override
77-
public void surfaceDestroyed() {
78-
onSurfaceDestroy(mContextId);
79-
}
80-
8176
@Override
8277
public void surfaceOffscreen() {
8378
switchToOffscreenSurface(mContextId);
8479
}
8580

81+
/**
82+
* Called from WebGPUViewManager.onDropViewInstance when React removes this
83+
* view: the view dies with its Canvas, so it retires the registry entry.
84+
*/
85+
public void destroy() {
86+
onViewDestroyed(mContextId);
87+
}
88+
8689
@DoNotStrip
8790
private native void onSurfaceCreate(
8891
Surface surface,
@@ -100,9 +103,9 @@ private native void onSurfaceChanged(
100103
);
101104

102105
@DoNotStrip
103-
private native void onSurfaceDestroy(int contextId);
106+
private native void switchToOffscreenSurface(int contextId);
104107

105108
@DoNotStrip
106-
private native void switchToOffscreenSurface(int contextId);
109+
private native void onViewDestroyed(int contextId);
107110

108111
}

packages/webgpu/android/src/main/java/com/webgpu/WebGPUViewManager.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ public WebGPUView createViewInstance(@NonNull ThemedReactContext context) {
2424
return new WebGPUView(context);
2525
}
2626

27+
@Override
28+
public void onDropViewInstance(@NonNull WebGPUView view) {
29+
super.onDropViewInstance(view);
30+
view.destroy();
31+
}
32+
2733
@Override
2834
@ReactProp(name = "transparent")
2935
public void setTransparent(WebGPUView view, boolean value) {

packages/webgpu/apple/MetalView.mm

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,52 @@ - (instancetype)init {
2323
- (void)configure {
2424
auto size = self.frame.size;
2525
std::shared_ptr<rnwgpu::RNWebGPUManager> manager = [WebGPUModule getManager];
26-
void *nativeSurface = (__bridge void *)self.layer;
26+
if (manager == nullptr) {
27+
return;
28+
}
29+
// Retain the layer for as long as SurfaceInfo holds the pointer: the
30+
// latched attach (and the flush lambda that adopts it) can outlive this
31+
// view, e.g. across a dev reload where the registry is cleared before
32+
// dealloc runs. Balanced by the releaser below.
33+
void *nativeSurface = (void *)CFBridgingRetain(self.layer);
2734
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
2835
auto gpu = manager->_gpu;
2936
auto surface = manager->_platformContext->makeSurface(
3037
gpu, nativeSurface, size.width, size.height);
31-
registry
32-
.getSurfaceInfoOrCreate([_contextId intValue], gpu, size.width,
33-
size.height)
34-
->switchToOnscreen(nativeSurface, surface);
38+
// Find-or-create + attach runs atomically under the registry lock so a
39+
// concurrent destroyContext cannot orphan this surface.
40+
auto info = registry.attachSurface(
41+
[_contextId intValue], gpu, size.width, size.height, nativeSurface,
42+
surface, [](void *layer) {
43+
// The releaser can run on the rendering thread; CALayer teardown
44+
// belongs on the main thread.
45+
dispatch_async(dispatch_get_main_queue(), ^{
46+
CFBridgingRelease(layer);
47+
});
48+
});
49+
// The attach is adopted at the next frame boundary by the rendering thread;
50+
// schedule a flush so contexts that are not currently rendering still pick
51+
// it up (and present their last offscreen frame).
52+
manager->flushPendingSurfaceTransition(info);
3553
}
3654

3755
- (void)update {
3856
auto size = self.frame.size;
3957
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
40-
registry.getSurfaceInfo([_contextId intValue])
41-
->resize(size.width, size.height);
58+
if (auto info = registry.getSurfaceInfo([_contextId intValue])) {
59+
info->resize(size.width, size.height);
60+
}
4261
}
4362

4463
- (void)dealloc {
64+
// The view dies with its Canvas (contextIds are never reused), so view
65+
// teardown retires the registry entry. The JS-side cleanup
66+
// (RNWebGPU.destroyContext) only handles entries that never had a native
67+
// surface; see RNWebGPU::destroyContext for the ownership split.
4568
auto &registry = rnwgpu::SurfaceRegistry::getInstance();
46-
// Remove the surface info from the registry
69+
if (auto info = registry.getSurfaceInfo([_contextId intValue])) {
70+
info->detachSurface();
71+
}
4772
registry.removeSurfaceInfo([_contextId intValue]);
4873
}
4974

packages/webgpu/cpp/rnwgpu/RNWebGPUManager.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@
3131
#include "GPURenderPassEncoder.h"
3232
#include "GPURenderPipeline.h"
3333
#include "GPUSampler.h"
34+
#include "GPUShaderModule.h"
3435
#include "GPUSharedFence.h"
3536
#include "GPUSharedTextureMemory.h"
36-
#include "GPUShaderModule.h"
3737
#include "GPUSupportedLimits.h"
3838
#include "GPUTexture.h"
3939
#include "GPUTextureView.h"
@@ -237,7 +237,20 @@ void RNWebGPUManager::installWebGPUWorkletHelpers(jsi::Runtime &runtime) {
237237
runtime.global().setProperty(runtime, "__webgpuBox", std::move(boxFunc));
238238
}
239239

240+
void RNWebGPUManager::flushPendingSurfaceTransition(
241+
std::shared_ptr<SurfaceInfo> info) {
242+
if (info == nullptr || _jsCallInvoker == nullptr) {
243+
return;
244+
}
245+
_jsCallInvoker->invokeAsync(
246+
[info = std::move(info)] { info->applyPendingAttach(); });
247+
}
248+
240249
RNWebGPUManager::~RNWebGPUManager() {
250+
// Drop all canvas registry entries: after a reload the JS side restarts its
251+
// contextId counter, and stale entries would alias new canvases onto dead
252+
// surfaces.
253+
SurfaceRegistry::getInstance().clear();
241254
_jsRuntime = nullptr;
242255
_jsCallInvoker = nullptr;
243256
}

0 commit comments

Comments
 (0)