From 72e278ffac760751b9d5104e5598612f999f7805 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 9 Sep 2021 12:13:54 +0000 Subject: [PATCH 1/5] Add escape hatch for old environments --- .../Sources/PrimaryTests/main.swift | 36 ++++- Runtime/src/index.ts | 57 +++++-- Sources/JavaScriptKit/Features.swift | 12 ++ .../FundamentalObjects/JSClosure.swift | 146 +++++++++--------- Sources/_CJavaScriptKit/_CJavaScriptKit.c | 9 +- 5 files changed, 172 insertions(+), 88 deletions(-) create mode 100644 Sources/JavaScriptKit/Features.swift diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index 8d32792e5..2d9b5572a 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -197,18 +197,26 @@ try test("Closure Lifetime") { return arguments[0] } try expectEqual(evalClosure(c1, JSValue.number(1.0)), .number(1.0)) +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + c1.release() +#endif } do { let c1 = JSClosure { _ in .undefined } +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS c1.release() - c1.release() +#endif } do { let array = JSObject.global.Array.function!.new() - _ = array.push!(JSClosure { _ in .number(3) }) + let c1 = JSClosure { _ in .number(3) } + _ = array.push!(c1) try expectEqual(array[0].function!().number, 3.0) +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + c1.release() +#endif } // do { @@ -221,6 +229,7 @@ try test("Closure Lifetime") { // try expectEqual(weakRef.deref!(), .undefined) // } +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS do { let c1 = JSOneshotClosure { _ in return .boolean(true) @@ -230,6 +239,7 @@ try test("Closure Lifetime") { try expectCrashByCall(ofClosure: c1) // OneshotClosure won't call fatalError even if it's deallocated before `release` } +#endif } try test("Host Function Registration") { @@ -261,6 +271,10 @@ try test("Host Function Registration") { try expectEqual(call_host_1Func(), .number(1)) try expectEqual(isHostFunc1Called, true) +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + hostFunc1.release() +#endif + let hostFunc2 = JSClosure { (arguments) -> JSValue in do { let input = try expectNumber(arguments[0]) @@ -272,6 +286,10 @@ try test("Host Function Registration") { try expectEqual(evalClosure(hostFunc2, 3), .number(6)) _ = try expectString(evalClosure(hostFunc2, true)) + +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + hostFunc2.release() +#endif } try test("New Object Construction") { @@ -380,14 +398,24 @@ try test("ObjectRef Lifetime") { // } // ``` + let identity = JSClosure { $0[0] } let ref1 = getJSValue(this: .global, name: "globalObject1").object! - let ref2 = evalClosure(JSClosure { $0[0] }, ref1).object! + let ref2 = evalClosure(identity, ref1).object! try expectEqual(ref1.prop_2, .number(2)) try expectEqual(ref2.prop_2, .number(2)) + +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + identity.release() +#endif } func closureScope() -> ObjectIdentifier { - ObjectIdentifier(JSClosure { _ in .undefined }) + let closure = JSClosure { _ in .undefined } + let result = ObjectIdentifier(closure) +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS + closure.release() +#endif + return result } try test("Closure Identifiers") { diff --git a/Runtime/src/index.ts b/Runtime/src/index.ts index 3b99447e7..46257897e 100644 --- a/Runtime/src/index.ts +++ b/Runtime/src/index.ts @@ -21,6 +21,7 @@ if (typeof globalThis !== "undefined") { interface SwiftRuntimeExportedFunctions { swjs_library_version(): number; + swjs_library_features(): number; swjs_prepare_host_function_call(size: number): pointer; swjs_cleanup_host_function_call(argv: pointer): void; swjs_call_host_function( @@ -44,6 +45,10 @@ enum JavaScriptValueKind { Function = 6, } +enum LibraryFeatures { + WeakRefs = 1 << 0, +} + type TypedArray = | Int8ArrayConstructor | Uint8ArrayConstructor @@ -117,25 +122,33 @@ class SwiftRuntimeHeap { } } +/// Memory lifetime of closures in Swift are managed by Swift side +class SwiftClosureHeap { + private functionRegistry: FinalizationRegistry; + private exports: SwiftRuntimeExportedFunctions + + constructor(exports: SwiftRuntimeExportedFunctions) { + this.exports = exports + this.functionRegistry = new FinalizationRegistry((id) => { + this.exports.swjs_free_host_function(id); + }); + } + + alloc(func: any, func_ref: number) { + this.functionRegistry.register(func, func_ref); + } +} + export class SwiftRuntime { private instance: WebAssembly.Instance | null; private heap: SwiftRuntimeHeap; - private functionRegistry: FinalizationRegistry; + private _closureHeap: SwiftClosureHeap | null; private version: number = 701; constructor() { this.instance = null; this.heap = new SwiftRuntimeHeap(); - this.functionRegistry = new FinalizationRegistry( - this.handleFree.bind(this) - ); - } - - handleFree(id: unknown) { - if (!this.instance || typeof id !== "number") return; - const exports = (this.instance - .exports as any) as SwiftRuntimeExportedFunctions; - exports.swjs_free_host_function(id); + this._closureHeap = null; } setInstance(instance: WebAssembly.Instance) { @@ -146,6 +159,26 @@ export class SwiftRuntime { throw new Error("The versions of JavaScriptKit are incompatible."); } } + get closureHeap(): SwiftClosureHeap | null { + if (this._closureHeap) + return this._closureHeap; + if (!this.instance) + throw new Error("WebAssembly instance is not set yet"); + + const exports = (this.instance + .exports as any) as SwiftRuntimeExportedFunctions; + const features = exports.swjs_library_features(); + const librarySupportsWeakRef = (features & LibraryFeatures.WeakRefs) != 0; + if (librarySupportsWeakRef) { + if (typeof FinalizationRegistry !== "undefined") { + this._closureHeap = new SwiftClosureHeap(exports); + return this._closureHeap; + } else { + throw new Error("The JavaScriptKit in Swift expects the target environment supports WeakRefs. Please build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` to disable features using WeakRefs."); + } + } + return null; + } importObjects() { const memory = () => { @@ -472,7 +505,7 @@ export class SwiftRuntime { ); }; const func_ref = this.heap.retain(func); - this.functionRegistry.register(func, func_ref); + this.closureHeap?.alloc(func, func_ref); writeUint32(func_ref_ptr, func_ref); }, swjs_call_throwing_new: ( diff --git a/Sources/JavaScriptKit/Features.swift b/Sources/JavaScriptKit/Features.swift new file mode 100644 index 000000000..e479003c5 --- /dev/null +++ b/Sources/JavaScriptKit/Features.swift @@ -0,0 +1,12 @@ +enum LibraryFeatures { + static let weakRefs: Int32 = 1 << 0 +} + +@_cdecl("_library_features") +func _library_features() -> Int32 { + var features: Int32 = 0 +#if !JAVASCRIPTKIT_WITHOUT_WEAKREFS + features |= LibraryFeatures.weakRefs +#endif + return features +} diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index f8411528d..e34d538c0 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -1,9 +1,6 @@ import _CJavaScriptKit -fileprivate var closureRef: JavaScriptHostFuncRef = 0 -fileprivate var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] - -/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime is manualy managed +/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime might be manualy managed public protocol JSClosureProtocol: JSValueCompatible { /// Release this function resource. @@ -12,6 +9,35 @@ public protocol JSClosureProtocol: JSValueCompatible { } +/// `JSOneshotClosure` is a JavaScript function that can be called only once. +public class JSOneshotClosure: JSObject, JSClosureProtocol { + private var hostFuncRef: JavaScriptHostFuncRef = 0 + + public init(_ body: @escaping ([JSValue]) -> JSValue) { + // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. + super.init(id: 0) + let objectId = ObjectIdentifier(self) + let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) + // 2. Retain the given body in static storage by `funcRef`. + JSClosure.sharedClosures[funcRef] = { + defer { self.release() } + return body($0) + } + // 3. Create a new JavaScript function which calls the given Swift function. + var objectRef: JavaScriptObjectRef = 0 + _create_function(funcRef, &objectRef) + + hostFuncRef = funcRef + id = objectRef + } + + /// Release this function resource. + /// After calling `release`, calling this function from JavaScript will fail. + public func release() { + JSClosure.sharedClosures[hostFuncRef] = nil + } +} + /// `JSClosure` represents a JavaScript function the body of which is written in Swift. /// This type can be passed as a callback handler to JavaScript functions. /// @@ -28,8 +54,15 @@ public protocol JSClosureProtocol: JSValueCompatible { /// ``` /// public class JSClosure: JSObject, JSClosureProtocol { + + fileprivate static var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] + private var hostFuncRef: JavaScriptHostFuncRef = 0 + #if JAVASCRIPTKIT_WITHOUT_WEAKREFS + private var isReleased: Bool = false + #endif + @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") @_disfavoredOverload public convenience init(_ body: @escaping ([JSValue]) -> ()) { @@ -40,26 +73,27 @@ public class JSClosure: JSObject, JSClosureProtocol { } public init(_ body: @escaping ([JSValue]) -> JSValue) { - self.hostFuncRef = closureRef - closureRef += 1 - - // Retain the given body in static storage by `closureRef`. - sharedClosures[self.hostFuncRef] = body - - // Create a new JavaScript function which calls the given Swift function. + // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. + super.init(id: 0) + let objectId = ObjectIdentifier(self) + let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) + // 2. Retain the given body in static storage by `funcRef`. + Self.sharedClosures[funcRef] = body + // 3. Create a new JavaScript function which calls the given Swift function. var objectRef: JavaScriptObjectRef = 0 - _create_function(self.hostFuncRef, &objectRef) + _create_function(funcRef, &objectRef) - super.init(id: objectRef) + hostFuncRef = funcRef + id = objectRef } - @available(*, deprecated, message: "JSClosure.release() is no longer necessary") - public func release() {} -} - -@_cdecl("_free_host_function_impl") -func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) { - sharedClosures[hostFuncRef] = nil + #if JAVASCRIPTKIT_WITHOUT_WEAKREFS + deinit { + guard isReleased else { + fatalError("release() must be called on JSClosure objects manually before they are deallocated") + } + } + #endif } @@ -103,7 +137,7 @@ func _call_host_function_impl( _ argv: UnsafePointer, _ argc: Int32, _ callbackFuncRef: JavaScriptObjectRef ) { - guard let hostFunc = sharedClosures[hostFuncRef] else { + guard let hostFunc = JSClosure.sharedClosures[hostFuncRef] else { fatalError("The function was already released") } let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map { @@ -114,66 +148,36 @@ func _call_host_function_impl( _ = callbackFuncRef(result) } -// MARK: - Legacy Closure Types -/// `JSOneshotClosure` is a JavaScript function that can be called only once. -/// It is recommended to use `JSClosure` instead if your target runtimes support `FinalizationRegistry`. -public class JSOneshotClosure: JSObject, JSClosureProtocol { - private var hostFuncRef: JavaScriptHostFuncRef = 0 - public init(_ body: @escaping ([JSValue]) -> JSValue) { - // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. - super.init(id: 0) - let objectId = ObjectIdentifier(self) - let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) - // 2. Retain the given body in static storage by `funcRef`. - sharedClosures[funcRef] = { - defer { self.release() } - return body($0) - } - // 3. Create a new JavaScript function which calls the given Swift function. - var objectRef: JavaScriptObjectRef = 0 - _create_function(funcRef, &objectRef) +// [WeakRefs](https://github.com/tc39/proposal-weakrefs) is already Stage 4, but it's still newish API, +// so provide a escape hatch. +// Please build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` by SwiftPM build system +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS - hostFuncRef = funcRef - id = objectRef - } +// MARK: - Legacy Closure Types - /// Release this function resource. - /// After calling `release`, calling this function from JavaScript will fail. +extension JSClosure { public func release() { - sharedClosures[hostFuncRef] = nil + isReleased = true + Self.sharedClosures[hostFuncRef] = nil } } -public class JSUnretainedClosure: JSObject, JSClosureProtocol { - private var hostFuncRef: JavaScriptHostFuncRef = 0 - var isReleased: Bool = false +@_cdecl("_free_host_function_impl") +func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) {} - public init(_ body: @escaping ([JSValue]) -> JSValue) { - // 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`. - super.init(id: 0) - let objectId = ObjectIdentifier(self) - let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) - // 2. Retain the given body in static storage by `funcRef`. - sharedClosures[funcRef] = body - // 3. Create a new JavaScript function which calls the given Swift function. - var objectRef: JavaScriptObjectRef = 0 - _create_function(funcRef, &objectRef) +#else - hostFuncRef = funcRef - id = objectRef - } +extension JSClosure { - public func release() { - isReleased = true - sharedClosures[hostFuncRef] = nil - } + @available(*, deprecated, message: "JSClosure.release() is no longer necessary if the target environment supports WeakRefs") + public func release() {} - deinit { - guard isReleased else { - // Safari doesn't support `FinalizationRegistry`, so we cannot automatically manage the lifetime of Swift objects - fatalError("release() must be called on JSClosure objects manually before they are deallocated") - } - } } + +@_cdecl("_free_host_function_impl") +func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) { + JSClosure.sharedClosures[hostFuncRef] = nil +} +#endif diff --git a/Sources/_CJavaScriptKit/_CJavaScriptKit.c b/Sources/_CJavaScriptKit/_CJavaScriptKit.c index 8834cd214..9ad9aa003 100644 --- a/Sources/_CJavaScriptKit/_CJavaScriptKit.c +++ b/Sources/_CJavaScriptKit/_CJavaScriptKit.c @@ -35,8 +35,15 @@ void swjs_cleanup_host_function_call(void *argv_buffer) { /// Notes: If you change any interface of runtime library, please increment /// this and `SwiftRuntime.version` in `./Runtime/src/index.ts`. __attribute__((export_name("swjs_library_version"))) -int swjs_library_version() { +int swjs_library_version(void) { return 701; } +int _library_features(void); + +__attribute__((export_name("swjs_library_features"))) +int swjs_library_features(void) { + return _library_features(); +} + #endif From 592853fed63838c71c2b87f91199477203a09a86 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 9 Sep 2021 06:42:50 -0700 Subject: [PATCH 2/5] Run integration tests for JAVASCRIPTKIT_WITHOUT_WEAKREFS mode --- IntegrationTests/Makefile | 4 +++- Makefile | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/IntegrationTests/Makefile b/IntegrationTests/Makefile index 66eeda4c6..cdb295704 100644 --- a/IntegrationTests/Makefile +++ b/IntegrationTests/Makefile @@ -1,11 +1,13 @@ CONFIGURATION ?= debug +SWIFT_BUILD_FLAGS ?= FORCE: TestSuites/.build/$(CONFIGURATION)/%.wasm: FORCE swift build --package-path TestSuites \ --product $(basename $(notdir $@)) \ --triple wasm32-unknown-wasi \ - --configuration $(CONFIGURATION) + --configuration $(CONFIGURATION) \ + $(SWIFT_BUILD_FLAGS) dist/%.wasm: TestSuites/.build/$(CONFIGURATION)/%.wasm mkdir -p dist diff --git a/Makefile b/Makefile index b80f301aa..b48af9e2b 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,9 @@ build: test: cd IntegrationTests && \ CONFIGURATION=debug make test && \ - CONFIGURATION=release make test + CONFIGURATION=debug SWIFT_BUILD_FLAGS="-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS" make test && \ + CONFIGURATION=release make test && \ + CONFIGURATION=release SWIFT_BUILD_FLAGS="-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS" make test .PHONY: benchmark_setup benchmark_setup: From 7aa8158748ba535f3455542c4b1b2548a402be75 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 9 Sep 2021 16:38:55 -0700 Subject: [PATCH 3/5] Apply format changes --- Runtime/src/index.ts | 18 +++++++++--------- .../FundamentalObjects/JSClosure.swift | 13 +++++++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/Runtime/src/index.ts b/Runtime/src/index.ts index 46257897e..abed68209 100644 --- a/Runtime/src/index.ts +++ b/Runtime/src/index.ts @@ -125,12 +125,10 @@ class SwiftRuntimeHeap { /// Memory lifetime of closures in Swift are managed by Swift side class SwiftClosureHeap { private functionRegistry: FinalizationRegistry; - private exports: SwiftRuntimeExportedFunctions constructor(exports: SwiftRuntimeExportedFunctions) { - this.exports = exports this.functionRegistry = new FinalizationRegistry((id) => { - this.exports.swjs_free_host_function(id); + exports.swjs_free_host_function(id); }); } @@ -160,24 +158,26 @@ export class SwiftRuntime { } } get closureHeap(): SwiftClosureHeap | null { - if (this._closureHeap) - return this._closureHeap; + if (this._closureHeap) return this._closureHeap; if (!this.instance) throw new Error("WebAssembly instance is not set yet"); const exports = (this.instance .exports as any) as SwiftRuntimeExportedFunctions; const features = exports.swjs_library_features(); - const librarySupportsWeakRef = (features & LibraryFeatures.WeakRefs) != 0; + const librarySupportsWeakRef = + (features & LibraryFeatures.WeakRefs) != 0; if (librarySupportsWeakRef) { if (typeof FinalizationRegistry !== "undefined") { this._closureHeap = new SwiftClosureHeap(exports); - return this._closureHeap; + return this._closureHeap; } else { - throw new Error("The JavaScriptKit in Swift expects the target environment supports WeakRefs. Please build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` to disable features using WeakRefs."); + throw new Error( + "The Swift part of JavaScriptKit was configured to require the availability of JavaScript WeakRefs. Please build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` to disable features that use WeakRefs." + ); } } - return null; + return null; } importObjects() { diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index e34d538c0..9fc3c1768 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -1,6 +1,8 @@ import _CJavaScriptKit -/// JSClosureProtocol abstracts closure object in JavaScript, whose lifetime might be manualy managed +/// JSClosureProtocol wraps Swift closure objects for use in JavaScript. Conforming types +/// are responsible for managing the lifetime of the closure they wrap, but can delegate that +/// task to the user by requiring an explicit `release()` call. public protocol JSClosureProtocol: JSValueCompatible { /// Release this function resource. @@ -149,10 +151,9 @@ func _call_host_function_impl( } - -// [WeakRefs](https://github.com/tc39/proposal-weakrefs) is already Stage 4, but it's still newish API, -// so provide a escape hatch. -// Please build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` by SwiftPM build system +/// [WeakRefs](https://github.com/tc39/proposal-weakrefs) are already Stage 4, +/// but was added recently enough that older browser versions don’t support it. +/// Build with `-Xswiftc -DJAVASCRIPTKIT_WITHOUT_WEAKREFS` to disable the relevant behavior. #if JAVASCRIPTKIT_WITHOUT_WEAKREFS // MARK: - Legacy Closure Types @@ -171,7 +172,7 @@ func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) {} extension JSClosure { - @available(*, deprecated, message: "JSClosure.release() is no longer necessary if the target environment supports WeakRefs") + @available(*, deprecated, message: "JSClosure.release() is no longer necessary") public func release() {} } From 728443580c81afc40e6b89ef4c52350854f3dacc Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 9 Sep 2021 16:40:40 -0700 Subject: [PATCH 4/5] Bump Runtime version --- Runtime/src/index.ts | 2 +- Sources/_CJavaScriptKit/_CJavaScriptKit.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Runtime/src/index.ts b/Runtime/src/index.ts index abed68209..a48e8fa72 100644 --- a/Runtime/src/index.ts +++ b/Runtime/src/index.ts @@ -141,7 +141,7 @@ export class SwiftRuntime { private instance: WebAssembly.Instance | null; private heap: SwiftRuntimeHeap; private _closureHeap: SwiftClosureHeap | null; - private version: number = 701; + private version: number = 702; constructor() { this.instance = null; diff --git a/Sources/_CJavaScriptKit/_CJavaScriptKit.c b/Sources/_CJavaScriptKit/_CJavaScriptKit.c index 9ad9aa003..98f444fea 100644 --- a/Sources/_CJavaScriptKit/_CJavaScriptKit.c +++ b/Sources/_CJavaScriptKit/_CJavaScriptKit.c @@ -36,7 +36,7 @@ void swjs_cleanup_host_function_call(void *argv_buffer) { /// this and `SwiftRuntime.version` in `./Runtime/src/index.ts`. __attribute__((export_name("swjs_library_version"))) int swjs_library_version(void) { - return 701; + return 702; } int _library_features(void); From ab2ae19ecf578270453d736c09f14ba24c033cc4 Mon Sep 17 00:00:00 2001 From: Yuta Saito Date: Thu, 9 Sep 2021 21:11:40 -0700 Subject: [PATCH 5/5] Fix closure object identity --- .../TestSuites/Sources/PrimaryTests/main.swift | 4 ++-- .../JavaScriptKit/FundamentalObjects/JSClosure.swift | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift index 2d9b5572a..6c054a791 100644 --- a/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift +++ b/IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift @@ -409,12 +409,11 @@ try test("ObjectRef Lifetime") { #endif } +#if JAVASCRIPTKIT_WITHOUT_WEAKREFS func closureScope() -> ObjectIdentifier { let closure = JSClosure { _ in .undefined } let result = ObjectIdentifier(closure) -#if JAVASCRIPTKIT_WITHOUT_WEAKREFS closure.release() -#endif return result } @@ -423,6 +422,7 @@ try test("Closure Identifiers") { let oid2 = closureScope() try expectEqual(oid1, oid2) } +#endif func checkArray(_ array: [T]) throws where T: TypedArrayElement { try expectEqual(toString(JSTypedArray(array).jsValue().object!), jsStringify(array)) diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift index 9fc3c1768..a8fcd01e9 100644 --- a/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift +++ b/Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift @@ -21,10 +21,10 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol { let objectId = ObjectIdentifier(self) let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) // 2. Retain the given body in static storage by `funcRef`. - JSClosure.sharedClosures[funcRef] = { + JSClosure.sharedClosures[funcRef] = (self, { defer { self.release() } return body($0) - } + }) // 3. Create a new JavaScript function which calls the given Swift function. var objectRef: JavaScriptObjectRef = 0 _create_function(funcRef, &objectRef) @@ -57,7 +57,8 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol { /// public class JSClosure: JSObject, JSClosureProtocol { - fileprivate static var sharedClosures: [JavaScriptHostFuncRef: ([JSValue]) -> JSValue] = [:] + // Note: Retain the closure object itself also to avoid funcRef conflicts + fileprivate static var sharedClosures: [JavaScriptHostFuncRef: (object: JSObject, body: ([JSValue]) -> JSValue)] = [:] private var hostFuncRef: JavaScriptHostFuncRef = 0 @@ -80,7 +81,7 @@ public class JSClosure: JSObject, JSClosureProtocol { let objectId = ObjectIdentifier(self) let funcRef = JavaScriptHostFuncRef(bitPattern: Int32(objectId.hashValue)) // 2. Retain the given body in static storage by `funcRef`. - Self.sharedClosures[funcRef] = body + Self.sharedClosures[funcRef] = (self, body) // 3. Create a new JavaScript function which calls the given Swift function. var objectRef: JavaScriptObjectRef = 0 _create_function(funcRef, &objectRef) @@ -139,7 +140,7 @@ func _call_host_function_impl( _ argv: UnsafePointer, _ argc: Int32, _ callbackFuncRef: JavaScriptObjectRef ) { - guard let hostFunc = JSClosure.sharedClosures[hostFuncRef] else { + guard let (_, hostFunc) = JSClosure.sharedClosures[hostFuncRef] else { fatalError("The function was already released") } let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map {