Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 8780e5f

Browse files
Revert "Fix JSClosure leak (#240)"
This reverts commit d9a4a9f.
1 parent d9a4a9f commit 8780e5f

File tree

4 files changed

+28
-103
lines changed

4 files changed

+28
-103
lines changed

‎IntegrationTests/TestSuites/Sources/ConcurrencyTests/main.swift‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,12 @@ func entrypoint() async throws {
123123
try expectEqual(result, .number(3))
124124
}
125125
try expectGTE(diff, 200)
126-
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
127-
delayObject.closure = nil
128-
delayClosure.release()
129-
#endif
130126
}
131127

132128
try await asyncTest("Async JSPromise: then") {
133129
let promise = JSPromise { resolve in
134130
_ = JSObject.global.setTimeout!(
135-
JSOneshotClosure { _ in
131+
JSClosure { _ in
136132
resolve(.success(JSValue.number(3)))
137133
return .undefined
138134
}.jsValue,
@@ -153,7 +149,7 @@ func entrypoint() async throws {
153149
try await asyncTest("Async JSPromise: then(success:failure:)") {
154150
let promise = JSPromise { resolve in
155151
_ = JSObject.global.setTimeout!(
156-
JSOneshotClosure { _ in
152+
JSClosure { _ in
157153
resolve(.failure(JSError(message: "test").jsValue))
158154
return .undefined
159155
}.jsValue,
@@ -172,7 +168,7 @@ func entrypoint() async throws {
172168
try await asyncTest("Async JSPromise: catch") {
173169
let promise = JSPromise { resolve in
174170
_ = JSObject.global.setTimeout!(
175-
JSOneshotClosure { _ in
171+
JSClosure { _ in
176172
resolve(.failure(JSError(message: "test").jsValue))
177173
return .undefined
178174
}.jsValue,

‎IntegrationTests/TestSuites/Sources/PrimaryTests/UnitTestUtils.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func expectThrow<T>(_ body: @autoclosure () throws -> T, file: StaticString = #f
111111
}
112112

113113
func wrapUnsafeThrowableFunction(_ body: @escaping () -> Void, file: StaticString = #file, line: UInt = #line, column: UInt = #column) throws -> Error {
114-
JSObject.global.callThrowingClosure.function!(JSOneshotClosure { _ in
114+
JSObject.global.callThrowingClosure.function!(JSClosure { _ in
115115
body()
116116
return .undefined
117117
})

‎IntegrationTests/TestSuites/Sources/PrimaryTests/main.swift‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -254,18 +254,12 @@ try test("Closure Lifetime") {
254254
do {
255255
let c1 = JSClosure { _ in .number(4) }
256256
try expectEqual(c1(), .number(4))
257-
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
258-
c1.release()
259-
#endif
260257
}
261258

262259
do {
263260
let c1 = JSClosure { _ in fatalError("Crash while closure evaluation") }
264261
let error = try expectThrow(try evalClosure.throws(c1)) as! JSValue
265262
try expectEqual(error.description, "RuntimeError: unreachable")
266-
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
267-
c1.release()
268-
#endif
269263
}
270264
}
271265

@@ -872,24 +866,16 @@ try test("Symbols") {
872866
// }.prop
873867
// Object.defineProperty(hasInstanceClass, Symbol.hasInstance, { value: () => true })
874868
let hasInstanceObject = JSObject.global.Object.function!.new()
875-
let hasInstanceObjectClosure = JSClosure { _ in .undefined }
876-
hasInstanceObject.prop = hasInstanceObjectClosure.jsValue
869+
hasInstanceObject.prop = JSClosure { _ in .undefined }.jsValue
877870
let hasInstanceClass = hasInstanceObject.prop.function!
878871
let propertyDescriptor = JSObject.global.Object.function!.new()
879-
let propertyDescriptorClosure = JSClosure { _ in .boolean(true) }
880-
propertyDescriptor.value = propertyDescriptorClosure.jsValue
872+
propertyDescriptor.value = JSClosure { _ in .boolean(true) }.jsValue
881873
_ = JSObject.global.Object.function!.defineProperty!(
882874
hasInstanceClass, JSSymbol.hasInstance,
883875
propertyDescriptor
884876
)
885877
try expectEqual(hasInstanceClass[JSSymbol.hasInstance].function!().boolean, true)
886878
try expectEqual(JSObject.global.Object.isInstanceOf(hasInstanceClass), true)
887-
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
888-
hasInstanceObject.prop = .undefined
889-
propertyDescriptor.value = .undefined
890-
hasInstanceObjectClosure.release()
891-
propertyDescriptorClosure.release()
892-
#endif
893879
}
894880

895881
struct AnimalStruct: Decodable {

‎Sources/JavaScriptKit/FundamentalObjects/JSClosure.swift‎

Lines changed: 22 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,17 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol {
1919
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
2020
super.init(id: 0)
2121

22-
// 2. Retain the given body in static storage
23-
// Leak the self object globally and release once it's called
24-
hostFuncRef = JSClosure.sharedClosures.register(ObjectIdentifier(self), object: .strong(self), body: {
25-
defer { self.release() }
26-
return body(0ドル)
27-
})
22+
// 2. Create a new JavaScript function which calls the given Swift function.
23+
hostFuncRef = JavaScriptHostFuncRef(bitPattern: ObjectIdentifier(self))
2824
id = withExtendedLifetime(JSString(file)) { file in
2925
_create_function(hostFuncRef, line, file.asInternalJSRef())
3026
}
27+
28+
// 3. Retain the given body in static storage by `funcRef`.
29+
JSClosure.sharedClosures[hostFuncRef] = (self, {
30+
defer { self.release() }
31+
return body(0ドル)
32+
})
3133
}
3234

3335
#if compiler(>=5.5)
@@ -40,7 +42,7 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol {
4042
/// Release this function resource.
4143
/// After calling `release`, calling this function from JavaScript will fail.
4244
public func release() {
43-
JSClosure.sharedClosures.unregister(hostFuncRef)
45+
JSClosure.sharedClosures[hostFuncRef]=nil
4446
}
4547
}
4648

@@ -60,13 +62,13 @@ public class JSOneshotClosure: JSObject, JSClosureProtocol {
6062
/// ```
6163
///
6264
public class JSClosure: JSFunction, JSClosureProtocol {
63-
fileprivate static var sharedClosures = SharedJSClosureRegistry()
65+
66+
// Note: Retain the closure object itself also to avoid funcRef conflicts
67+
fileprivate static var sharedClosures: [JavaScriptHostFuncRef: (object: JSObject, body: ([JSValue]) -> JSValue)] = [:]
6468

6569
private var hostFuncRef: JavaScriptHostFuncRef = 0
6670

6771
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
68-
private let file: String
69-
private let line: UInt32
7072
private var isReleased: Bool = false
7173
#endif
7274

@@ -80,35 +82,30 @@ public class JSClosure: JSFunction, JSClosureProtocol {
8082
}
8183

8284
public init(_ body: @escaping ([JSValue]) -> JSValue, file: String = #fileID, line: UInt32 = #line) {
83-
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
84-
self.file = file
85-
self.line = line
86-
#endif
8785
// 1. Fill `id` as zero at first to access `self` to get `ObjectIdentifier`.
8886
super.init(id: 0)
8987

90-
// 2. Retain the given body in static storage
91-
hostFuncRef = Self.sharedClosures.register(
92-
ObjectIdentifier(self), object: .weak(self), body: body
93-
)
94-
88+
// 2. Create a new JavaScript function which calls the given Swift function.
89+
hostFuncRef = JavaScriptHostFuncRef(bitPattern: ObjectIdentifier(self))
9590
id = withExtendedLifetime(JSString(file)) { file in
9691
_create_function(hostFuncRef, line, file.asInternalJSRef())
9792
}
9893

94+
// 3. Retain the given body in static storage by `funcRef`.
95+
Self.sharedClosures[hostFuncRef] = (self, body)
9996
}
10097

10198
#if compiler(>=5.5)
10299
@available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
103-
public static func async(_ body: @escaping ([JSValue]) async throws -> JSValue, file:String= #fileID, line:UInt32= #line) -> JSClosure {
104-
JSClosure(makeAsyncClosure(body), file: file, line: line)
100+
public static func async(_ body: @escaping ([JSValue]) async throws -> JSValue) -> JSClosure {
101+
JSClosure(makeAsyncClosure(body))
105102
}
106103
#endif
107104

108105
#if JAVASCRIPTKIT_WITHOUT_WEAKREFS
109106
deinit {
110107
guard isReleased else {
111-
fatalError("release() must be called on JSClosure object (\(file):\(line)) manually before they are deallocated")
108+
fatalError("release() must be called on JSClosure objects manually before they are deallocated")
112109
}
113110
}
114111
#endif
@@ -136,60 +133,6 @@ private func makeAsyncClosure(_ body: @escaping ([JSValue]) async throws -> JSVa
136133
}
137134
#endif
138135

139-
/// Registry for Swift closures that are referenced from JavaScript.
140-
private struct SharedJSClosureRegistry {
141-
struct ClosureEntry {
142-
// Note: Retain the closure object itself also to avoid funcRef conflicts.
143-
var object: AnyObjectReference
144-
var body: ([JSValue]) -> JSValue
145-
146-
init(object: AnyObjectReference, body: @escaping ([JSValue]) -> JSValue) {
147-
self.object = object
148-
self.body = body
149-
}
150-
}
151-
enum AnyObjectReference {
152-
case strong(AnyObject)
153-
case weak(WeakObject)
154-
155-
static func `weak`(_ object: AnyObject) -> AnyObjectReference {
156-
.weak(SharedJSClosureRegistry.WeakObject(underlying: object))
157-
}
158-
}
159-
struct WeakObject {
160-
weak var underlying: AnyObject?
161-
init(underlying: AnyObject) {
162-
self.underlying = underlying
163-
}
164-
}
165-
private var closures: [JavaScriptHostFuncRef: ClosureEntry] = [:]
166-
167-
/// Register a Swift closure to be called from JavaScript.
168-
/// - Parameters:
169-
/// - hint: A hint to identify the closure.
170-
/// - object: The object should be retained until the closure is released from JavaScript.
171-
/// - body: The closure to be called from JavaScript.
172-
/// - Returns: An unique identifier for the registered closure.
173-
mutating func register(
174-
_ hint: ObjectIdentifier,
175-
object: AnyObjectReference, body: @escaping ([JSValue]) -> JSValue
176-
) -> JavaScriptHostFuncRef {
177-
let ref = JavaScriptHostFuncRef(bitPattern: hint)
178-
closures[ref] = ClosureEntry(object: object, body: body)
179-
return ref
180-
}
181-
182-
/// Unregister a Swift closure from the registry.
183-
mutating func unregister(_ ref: JavaScriptHostFuncRef) {
184-
closures[ref] = nil
185-
}
186-
187-
/// Get the Swift closure from the registry.
188-
subscript(_ ref: JavaScriptHostFuncRef) -> (([JSValue]) -> JSValue)? {
189-
closures[ref]?.body
190-
}
191-
}
192-
193136
// MARK: - `JSClosure` mechanism note
194137
//
195138
// 1. Create a thunk in the JavaScript world, which has a reference
@@ -231,7 +174,7 @@ func _call_host_function_impl(
231174
_ argv: UnsafePointer<RawJSValue>, _ argc: Int32,
232175
_ callbackFuncRef: JavaScriptObjectRef
233176
) -> Bool {
234-
guard let hostFunc = JSClosure.sharedClosures[hostFuncRef] else {
177+
guard let (_,hostFunc) = JSClosure.sharedClosures[hostFuncRef] else {
235178
return true
236179
}
237180
let arguments = UnsafeBufferPointer(start: argv, count: Int(argc)).map(\.jsValue)
@@ -252,7 +195,7 @@ func _call_host_function_impl(
252195
extension JSClosure {
253196
public func release() {
254197
isReleased = true
255-
Self.sharedClosures.unregister(hostFuncRef)
198+
Self.sharedClosures[hostFuncRef]=nil
256199
}
257200
}
258201

@@ -270,6 +213,6 @@ extension JSClosure {
270213

271214
@_cdecl("_free_host_function_impl")
272215
func _free_host_function_impl(_ hostFuncRef: JavaScriptHostFuncRef) {
273-
JSClosure.sharedClosures.unregister(hostFuncRef)
216+
JSClosure.sharedClosures[hostFuncRef]=nil
274217
}
275218
#endif

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /