fix(resizer): guard dispose() against an in-flight resize() - #4130
Open
jslok wants to merge 1 commit into
Open
Conversation
resize() dereferenced the pipeline with no synchronization while dispose() could concurrently destroy it from the JS thread - a native use-after-free instead of the catchable "already been disposed" error. Serialize resize(), dispose() and the memorySize getter behind a lifecycle lock on both platforms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@jslok is attempting to deploy a commit to the Margelo Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #4117 (as requested).
resize()null-checks_pipelineand then dereferences it with no synchronization, whiledispose()can concurrently reset the sameunique_ptrfrom the JS thread — running~VulkanResizerPipeline()and tearing down the VkDevice under an in-flightrun(). That is a native use-after-free instead of the catchable "already been disposed" error. iOS has the same shape: an unguardedpipeline = nilracing theguard letload is a data race on the possibly-last reference.Fix: serialize
resize(),dispose()and thememorySizegetter behind a lifecycle lock (std::mutex/NSLock). A dispose landing mid-frame blocks until that resize returns; every later call throws the catchable error. The getter is included because Nitro reads it fromtoObject()on arbitrary threads; it is never called from insideresize()/dispose(), so it cannot self-deadlock.Tested on device on both platforms: on Android by disposing from an effect cleanup while frames were still being processed, and on iOS where this change has been shipping in our production app as a patch-package patch.
🤖 Generated with Claude Code