NotificationsYou must be signed in to change notification settings
Fork57
Star807

Fix wrong deallocation management for JSClosure with FinalizationRegistry#393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

kateinoigakukun merged 4 commits intomainfromyt/jsclosure-fix

Aug 4, 2025

Merged

Fix wrong deallocation management for JSClosure with FinalizationRegistry#393

kateinoigakukun merged 4 commits intomainfromyt/jsclosure-fix

Aug 4, 2025

Conversation

Copy link

Member

kateinoigakukun commentedAug 3, 2025•
edited
Loading

In FinalizationRegistry mode, theswjs_free_host_function should be called with the Swift heap object ID, not the JS closure object ID.
The wrong argument was causing:

Swift closures to be leaked, because FinalizationRegistry attempted to release a JS object ID that was never registered as a Swift closure.
Unintended deallocation of unrelated closures when a reused JS object ID happened to match a valid Swift closure ID by coincidence after so many JSObject allocations (likely 10~30 million allocations). This eventually led to crashes withThe JSClosure has been already released by Swift side.

Stop managing JSOneshotClosure by FinalizationRegistry

JSOneshotClosure manages its lifetime by itself, so it doesn't require
nondeterministic FinalizationRegistry. Additionally, use of FinalizationRegistry
was unsafe for the case:

JSOneshotClosure C1 is created
C1 is called, and destroyed
JSOneshotClosure C2 is created at the same address of C1
C1's finalizer callback is called

kateinoigakukun added4 commits

August 4, 2025 01:16

Add test case to kill the wrong deallocation issue of JSClosure

825f947

Fix wrong deallocation management for JSClosure with FinalizationRegi…

db3a6e2

…stryIn FinalizationRegistry mode, the `swjs_free_host_function` shouldbe called with the Swift heap object ID, not the JS closure object ID.The wrong argument was causing:- Swift closures to be leaked, because FinalizationRegistry attempted  to release a JS object ID that was never registered as a Swift closure.- Unintended deallocation of unrelated closures when a reused JS object ID  happened to match a valid Swift closure ID by coincidence. This  eventually led to crashes with `The JSClosure has been already released by Swift side.`

Stop managing JSOneshotClosure by FinalizationRegistry

7f9094e

JSOneshotClosure manages its lifetime by itself, so it doesn't requirenondeterministic FinalizationRegistry. Additionally, use of FinalizationRegistrywas unsafe for the case:1. JSOneshotClosure C1 is created2. C1 is called, and destroyed3. JSOneshotClosure C2 is created at the same address of C14. C1's finalizer callback is called

Add Swift closure heap address in the invalid closure reference error

5f01593