Skip to content

fix: crash calling OSR shared texture release() after texture GC'd#50473

Merged
MarshallOfSound merged 1 commit intomainfrom
sattard/fix-osr-shared-texture-release-crash
Mar 25, 2026
Merged

fix: crash calling OSR shared texture release() after texture GC'd#50473
MarshallOfSound merged 1 commit intomainfrom
sattard/fix-osr-shared-texture-release-crash

Conversation

@MarshallOfSound
Copy link
Copy Markdown
Member

The weak persistent tracking the OffscreenReleaseHolderMonitor was tied to the texture object, but the release() closure holds a raw pointer to the monitor via its v8::External data. If JS retained texture.release while dropping the texture itself, the monitor would be freed on GC and a later release() call would crash the main process.

Track the release function instead of the texture object. Since the texture holds release as a property, this keeps the monitor alive as long as either is reachable, while preserving the existing OSRSharedTextureNotReleased warning behavior.

Added a regression test that captures texture.release, forces GC on the texture, then invokes the stale closure.

Notes: Fixed a crash when calling an offscreen shared texture's release() after the texture object was garbage collected.

The weak persistent tracking the OffscreenReleaseHolderMonitor was tied
to the texture object, but the release() closure holds a raw pointer to
the monitor via its v8::External data. If JS retained texture.release
while dropping the texture itself, the monitor would be freed on GC and
a later release() call would crash.

Track the release function instead of the texture object. Since the
texture holds release as a property, this keeps the monitor alive as
long as either is reachable.
@MarshallOfSound MarshallOfSound added semver/patch backwards-compatible bug fixes target/39-x-y PR should also be added to the "39-x-y" branch. target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. labels Mar 25, 2026
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Mar 25, 2026
@reitowo
Copy link
Copy Markdown
Member

reitowo commented Mar 25, 2026

In JS, texture can be GC'd even if release function still has reference? First to know haha.

Copy link
Copy Markdown
Member

@reitowo reitowo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API LGTM

@MarshallOfSound MarshallOfSound merged commit 678adea into main Mar 25, 2026
86 checks passed
@MarshallOfSound MarshallOfSound deleted the sattard/fix-osr-shared-texture-release-crash branch March 25, 2026 17:48
@release-clerk
Copy link
Copy Markdown

release-clerk bot commented Mar 25, 2026

Release Notes Persisted

Fixed a crash when calling an offscreen shared texture's release() after the texture object was garbage collected.

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "39-x-y", please check out #50499

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "40-x-y", please check out #50500

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "41-x-y", please check out #50501

@trop trop bot removed the target/40-x-y PR should also be added to the "40-x-y" branch. label Mar 25, 2026
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "42-x-y", please check out #50502

@trop trop bot added merged/39-x-y PR was merged to the "39-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. merged/40-x-y PR was merged to the "40-x-y" branch. and removed target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. in-flight/39-x-y in-flight/42-x-y in-flight/41-x-y in-flight/40-x-y labels Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/39-x-y PR was merged to the "39-x-y" branch. merged/40-x-y PR was merged to the "40-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. new-pr 🌱 PR opened recently semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants