Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 4, 2025

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Aug 3, 2025

In FinalizationRegistry mode, the swjs_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 with The 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:

  1. JSOneshotClosure C1 is created
  2. C1 is called, and destroyed
  3. JSOneshotClosure C2 is created at the same address of C1
  4. C1's finalizer callback is called

…stry

In FinalizationRegistry mode, the `swjs_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. This
  eventually led to crashes with `The JSClosure has been already released by Swift side.`
JSOneshotClosure manages its lifetime by itself, so it doesn't require
nondeterministic FinalizationRegistry. Additionally, use of FinalizationRegistry
was unsafe for the case:
1. JSOneshotClosure C1 is created
2. C1 is called, and destroyed
3. JSOneshotClosure C2 is created at the same address of C1
4. C1's finalizer callback is called
@kateinoigakukun kateinoigakukun merged commit 4504e13 into main Aug 4, 2025
8 checks passed
@kateinoigakukun kateinoigakukun deleted the yt/jsclosure-fix branch August 4, 2025 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants