swiftwasm / JavaScriptKit

Swift framework to interact with JavaScript through WebAssembly.
https://swiftpackageindex.com/swiftwasm/JavaScriptKit/main/documentation/javascriptkit
MIT License
664 stars 44 forks source link

Fix `JSClosure` leak #240

Closed kateinoigakukun closed 5 months ago

kateinoigakukun commented 5 months ago
let c1 = JSClosure { _ in .undefined }
consume c1

did not release the JSClosure itself and it leaked the underlying JavaScript closure too because JSClosure -> JS closure thunk -> Closure registry entry -> JSClosure reference cycle was not broken when using FinalizationRegistry. (Without FR, it was broken by manual release call.)

Note that weakening the reference does not violates the contract that function reference should be unique because holding a weak reference does deinit but not deallocate the object, so ObjectIdentifier is not reused until the weak reference in the registry is removed.

github-actions[bot] commented 5 months ago

Time Change: +325ms (3%)

Total Time: 9,697ms

Test name Duration Change
Serialization/JavaScript function call through Wasm import 23ms +2ms (6%) 🔍
Serialization/JavaScript function call through Wasm import with int 16ms +2ms (9%) 🔍
Serialization/JavaScript function call from Swift 103ms +7ms (7%) 🔍
Serialization/JavaScript Number to Swift Int 331ms +32ms (9%) 🔍
View Unchanged | Test name | Duration | Change | | :--- | :---: | :---: | | Serialization/Swift Int to JavaScript with assignment | 334ms | +17ms (4%) | | Serialization/Swift Int to JavaScript with call | 993ms | +32ms (3%) | | Serialization/Swift String to JavaScript with assignment | 391ms | +8ms (2%) | | Serialization/Swift String to JavaScript with call | 1,048ms | +30ms (2%) | | Serialization/JavaScript String to Swift String | 3,821ms | +158ms (4%) | | Object heap/Increment and decrement RC | 2,625ms | +37ms (1%) |
View Baselines | Test name | Duration | | :--- | :---: | | Serialization/Call JavaScript function directly | 3ms | | Serialization/Assign JavaScript number directly | 3ms | | Serialization/Call with JavaScript number directly | 3ms | | Serialization/Write JavaScript string directly | 3ms | | Serialization/Call with JavaScript string directly | 3ms |
kateinoigakukun commented 5 months ago

Tested with a random large-scale application and no major regression found