Skip to content

Commit 36dd08e

Browse files
committed
Refactor JSClosure to leverage FinalizationRegistry
1 parent 62600b5 commit 36dd08e

File tree

9 files changed

+71
-125
lines changed

9 files changed

+71
-125
lines changed

IntegrationTests/TestSuites/Sources/BenchmarkTests/Benchmark.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@ class Benchmark {
1515
return .undefined
1616
}
1717
runner("\(title)/\(name)", jsBody)
18-
jsBody.release()
1918
}
2019
}

IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -191,36 +191,36 @@ try test("Closure Lifetime") {
191191
return arguments[0]
192192
}
193193
try expectEqual(evalClosure(c1, JSValue.number(1.0)), .number(1.0))
194-
c1.release()
195194
}
196195

197196
do {
198-
let c1 = JSClosure { arguments in
199-
return arguments[0]
200-
}
197+
let c1 = JSClosure { _ in .undefined }
198+
c1.release()
201199
c1.release()
202-
// Call a released closure
203-
_ = try expectThrow(try evalClosure.throws(c1))
204200
}
205201

206202
do {
207-
let c1 = JSClosure { _ in
208-
// JSClosure will be deallocated before `release()`
209-
_ = JSClosure { _ in .undefined }
210-
return .undefined
211-
}
212-
_ = try expectThrow(try evalClosure.throws(c1))
213-
c1.release()
203+
let array = JSObject.global.Array.function!.new()
204+
_ = array.push!(JSClosure { _ in .number(3) })
205+
try expectEqual(array[0].function!().number, 3.0)
214206
}
215207

208+
// do {
209+
// let weakRef = { () -> JSObject in
210+
// let c1 = JSClosure { _ in .undefined }
211+
// return JSObject.global.WeakRef.function!.new(c1)
212+
// }()
213+
//
214+
// // unsure if this will actually work since GC may not run immediately
215+
// try expectEqual(weakRef.deref!(), .undefined)
216+
// }
217+
216218
do {
217219
let c1 = JSOneshotClosure { _ in
218220
return .boolean(true)
219221
}
220222
try expectEqual(evalClosure(c1), .boolean(true))
221-
// second call will cause `fatalError` that can be caught as a JavaScript exception
222-
_ = try expectThrow(try evalClosure.throws(c1))
223-
// OneshotClosure won't call fatalError even if it's deallocated before `release`
223+
try expectEqual(evalClosure(c1), .boolean(true))
224224
}
225225
}
226226

@@ -253,8 +253,6 @@ try test("Host Function Registration") {
253253
try expectEqual(call_host_1Func(), .number(1))
254254
try expectEqual(isHostFunc1Called, true)
255255

256-
hostFunc1.release()
257-
258256
let hostFunc2 = JSClosure { (arguments) -> JSValue in
259257
do {
260258
let input = try expectNumber(arguments[0])
@@ -266,7 +264,6 @@ try test("Host Function Registration") {
266264

267265
try expectEqual(evalClosure(hostFunc2, 3), .number(6))
268266
_ = try expectString(evalClosure(hostFunc2, true))
269-
hostFunc2.release()
270267
}
271268

272269
try test("New Object Construction") {
@@ -375,19 +372,14 @@ try test("ObjectRef Lifetime") {
375372
// }
376373
// ```
377374

378-
let identity = JSClosure { $0[0] }
379375
let ref1 = getJSValue(this: .global, name: "globalObject1").object!
380-
let ref2 = evalClosure(identity, ref1).object!
376+
let ref2 = evalClosure(JSClosure { $0[0] }, ref1).object!
381377
try expectEqual(ref1.prop_2, .number(2))
382378
try expectEqual(ref2.prop_2, .number(2))
383-
identity.release()
384379
}
385380

386381
func closureScope() -> ObjectIdentifier {
387-
let closure = JSClosure { _ in .undefined }
388-
let result = ObjectIdentifier(closure)
389-
closure.release()
390-
return result
382+
ObjectIdentifier(JSClosure { _ in .undefined })
391383
}
392384

393385
try test("Closure Identifiers") {
@@ -513,7 +505,7 @@ try test("Timer") {
513505
interval = JSTimer(millisecondsDelay: 5, isRepeating: true) {
514506
// ensure that JSTimer is living
515507
try! expectNotNil(interval)
516-
// verify that at least `timeoutMilliseconds * count` passed since the `timeout`
508+
// verify that at least `timeoutMilliseconds * count` passed since the `timeout`
517509
// timer started
518510
try! expectEqual(start + timeoutMilliseconds * count <= JSDate().valueOf(), true)
519511

@@ -549,7 +541,8 @@ try test("Promise") {
549541
exp1.fulfill()
550542
return JSValue.undefined
551543
}
552-
.catch { _ -> JSValue in
544+
.catch { err -> JSValue in
545+
print(err.object!.stack.string!)
553546
fatalError("Not fired due to no throw")
554547
}
555548
.finally { exp1.fulfill() }

Runtime/src/index.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ interface SwiftRuntimeExportedFunctions {
2929
argc: number,
3030
callback_func_ref: ref
3131
): void;
32+
33+
swjs_free_host_function(host_func_id: number): void;
3234
}
3335

3436
enum JavaScriptValueKind {
@@ -118,11 +120,22 @@ class SwiftRuntimeHeap {
118120
export class SwiftRuntime {
119121
private instance: WebAssembly.Instance | null;
120122
private heap: SwiftRuntimeHeap;
123+
private functionRegistry: FinalizationRegistry;
121124
private version: number = 701;
122125

123126
constructor() {
124127
this.instance = null;
125128
this.heap = new SwiftRuntimeHeap();
129+
this.functionRegistry = new FinalizationRegistry(
130+
this.handleFree.bind(this)
131+
);
132+
}
133+
134+
handleFree(id: unknown) {
135+
if (!this.instance || typeof id !== "number") return;
136+
const exports = (this.instance
137+
.exports as any) as SwiftRuntimeExportedFunctions;
138+
exports.swjs_free_host_function(id);
126139
}
127140

128141
setInstance(instance: WebAssembly.Instance) {
@@ -452,12 +465,14 @@ export class SwiftRuntime {
452465
host_func_id: number,
453466
func_ref_ptr: pointer
454467
) => {
455-
const func_ref = this.heap.retain(function () {
468+
const func = function () {
456469
return callHostFunction(
457470
host_func_id,
458471
Array.prototype.slice.call(arguments)
459472
);
460-
});
473+
};
474+
const func_ref = this.heap.retain(func);
475+
this.functionRegistry.register(func, func_ref);
461476
writeUint32(func_ref_ptr, func_ref);
462477
},
463478
swjs_call_throwing_new: (

Runtime/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"rootDir": "src",
88
"strict": true,
99
"target": "es2017",
10+
"lib": ["es2017", "DOM", "ESNext.WeakRef"],
1011
"skipLibCheck": true
1112
},
1213
"include": ["src/**/*"],

Sources/JavaScriptKit/Deprecated.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,6 @@ public typealias JSValueConstructible = ConstructibleFromJSValue
1515

1616
@available(*, deprecated, renamed: "JSValueCompatible")
1717
public typealias JSValueCodable = JSValueCompatible
18+
19+
@available(*, deprecated, renamed: "JSClosure")
20+
public typealias JSOneshotClosure = JSClosure

Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift

Lines changed: 17 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import _CJavaScriptKit
22

3+
fileprivate var closureRef: JavaScriptHostFuncRef = 0
34
fileprivate var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:]
45

56
/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime is manualy managed
@@ -10,40 +11,9 @@ public protocol JSClosureProtocol: JSValueCompatible {
1011
func release()
1112
}
1213

13-
/// `JSOneshotClosure` is a JavaScript function that can be called only once.
14-
public class JSOneshotClosure: JSObject, JSClosureProtocol {
15-
private var hostFuncRef: JavaScriptHostFuncRef = 0
16-
17-
public init(_ body: @escaping ([JSValue]) -> JSValue) {
18-
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
19-
super.init(id: 0)
20-
let objectId = ObjectIdentifier(self)
21-
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue))
22-
// 2. Retain the given body in static storage by `funcRef`.
23-
sharedClosures[funcRef] = {
24-
defer { self.release() }
25-
return body($0)
26-
}
27-
// 3. Create a new JavaScript function which calls the given Swift function.
28-
var objectRef: JavaScriptObjectRef = 0
29-
_create_function(funcRef, &objectRef)
30-
31-
hostFuncRef = funcRef
32-
id = objectRef
33-
}
34-
35-
/// Release this function resource.
36-
/// After calling `release`, calling this function from JavaScript will fail.
37-
public func release() {
38-
sharedClosures[hostFuncRef] = nil
39-
}
40-
}
4114

4215
/// `JSClosure` represents a JavaScript function the body of which is written in Swift.
4316
/// This type can be passed as a callback handler to JavaScript functions.
44-
/// Note that the lifetime of `JSClosure` should be managed by users manually
45-
/// due to GC boundary between Swift and JavaScript.
46-
/// For further discussion, see also [swiftwasm/JavaScriptKit #33](https://github.com/swiftwasm/JavaScriptKit/pull/33)
4717
///
4818
/// e.g.
4919
/// ```swift
@@ -55,12 +25,10 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol {
5525
/// button.addEventListener!("click", JSValue.function(eventListenter))
5626
/// ...
5727
/// button.removeEventListener!("click", JSValue.function(eventListenter))
58-
/// eventListenter.release()
5928
/// ```
6029
///
6130
public class JSClosure: JSObject, JSClosureProtocol {
6231
private var hostFuncRef: JavaScriptHostFuncRef = 0
63-
var isReleased: Bool = false
6432

6533
@available(*, deprecated, message: "This initializer will be removed in the next minor version update. Please use `init(_ body: @escaping ([JSValue]) -> JSValue)` and add `return .undefined` to the end of your closure")
6634
@_disfavoredOverload
@@ -72,33 +40,29 @@ public class JSClosure: JSObject, JSClosureProtocol {
7240
}
7341

7442
public init(_ body: @escaping ([JSValue]) -> JSValue) {
75-
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
76-
super.init(id: 0)
77-
let objectId = ObjectIdentifier(self)
78-
let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue))
79-
// 2. Retain the given body in static storage by `funcRef`.
80-
sharedClosures[funcRef] = body
81-
// 3. Create a new JavaScript function which calls the given Swift function.
43+
self.hostFuncRef = closureRef
44+
closureRef += 1
45+
46+
// Retain the given body in static storage by `closureRef`.
47+
sharedClosures[self.hostFuncRef] = body
48+
49+
// Create a new JavaScript function which calls the given Swift function.
8250
var objectRef: JavaScriptObjectRef = 0
83-
_create_function(funcRef, &objectRef)
51+
_create_function(self.hostFuncRef, &objectRef)
8452

85-
hostFuncRef = funcRef
86-
id = objectRef
53+
super.init(id: objectRef)
8754
}
8855

89-
public func release() {
90-
isReleased = true
91-
sharedClosures[hostFuncRef] = nil
92-
}
56+
@available(*, deprecated, message: "JSClosure.release() is no longer necessary")
57+
public func release() {}
58+
}
9359

94-
deinit {
95-
guard isReleased else {
96-
// Safari doesn't support `FinalizationRegistry`, so we cannot automatically manage the lifetime of Swift objects
97-
fatalError("release() must be called on JSClosure objects manually before they are deallocated")
98-
}
99-
}
60+
@_cdecl("_free_host_function_impl")
61+
func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) {
62+
sharedClosures[hostFuncRef] = nil
10063
}
10164

65+
10266
// MARK: - `JSClosure` mechanism note
10367
//
10468
// 1. Create a thunk in the JavaScript world, which has a reference

Sources/_CJavaScriptKit/_CJavaScriptKit.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ void swjs_call_host_function(const JavaScriptHostFuncRef host_func_ref,
1414
_call_host_function_impl(host_func_ref, argv, argc, callback_func);
1515
}
1616

17+
void _free_host_function_impl(const JavaScriptHostFuncRef host_func_ref);
18+
19+
__attribute__((export_name("swjs_free_host_function")))
20+
void swjs_free_host_function(const JavaScriptHostFuncRef host_func_ref) {
21+
_free_host_function_impl(host_func_ref);
22+
}
23+
1724
__attribute__((export_name("swjs_prepare_host_function_call")))
1825
void *swjs_prepare_host_function_call(const int argc) {
1926
return malloc(argc * sizeof(RawJSValue));

package-lock.json

Lines changed: 4 additions & 40 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,6 @@
3232
"license": "MIT",
3333
"devDependencies": {
3434
"prettier": "2.1.2",
35-
"typescript": "^4.0.2"
35+
"typescript": "^4.2.4"
3636
}
3737
}

0 commit comments

Comments
 (0)