Skip to content

Commit 866ba61

Browse files
krodakkrodak
authored
Merge pull request swiftwasm#664 from swiftwasm/yt/fix-heapobj-ownership
BridgeJS: Fix memory management of SwiftHeapObject
2 parents cf89c5f + afc2088 commit 866ba61

File tree

6 files changed

+264
-59
lines changed

6 files changed

+264
-59
lines changed

‎Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift‎

Lines changed: 68 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ import BridgeJSUtilities
1010
public struct BridgeJSLink {
1111
var skeletons: [BridgeJSSkeleton] = []
1212
let sharedMemory: Bool
13+
/// Whether to track the lifetime of Swift objects.
14+
///
15+
/// This is useful for debugging memory issues.
16+
let enableLifetimeTracking: Bool = false
1317
private let namespaceBuilder = NamespaceBuilder()
1418
private let intrinsicRegistry = JSIntrinsicRegistry()
1519

1620
public init(
1721
skeletons: [BridgeJSSkeleton] = [],
18-
sharedMemory: Bool
22+
sharedMemory: Bool = false
1923
) {
2024
self.skeletons = skeletons
2125
self.sharedMemory = sharedMemory
@@ -50,37 +54,76 @@ public struct BridgeJSLink {
5054
}
5155
"""
5256

53-
let swiftHeapObjectClassJs = """
54-
const swiftHeapObjectFinalizationRegistry = (typeof FinalizationRegistry === "undefined") ? { register: () => {}, unregister: () => {} } : new FinalizationRegistry((state) => {
55-
if (state.hasReleased) {
56-
return;
57-
}
58-
state.hasReleased = true;
59-
state.deinit(state.pointer);
60-
});
57+
let lifetimeTrackingClassJs = """
58+
const TRACKING = {
59+
wrap: (pointer, deinit, prototype, state) => {
60+
console.log(JSON.stringify({ DEBUG: true, event: "WRP", class: prototype.constructor.name, state }));
61+
},
62+
release: (obj) => {
63+
console.log(JSON.stringify({ DEBUG: true, event: "REL", class: obj.constructor.name, state: obj.__swiftHeapObjectState }));
64+
},
65+
finalization: (state) => {
66+
console.log(JSON.stringify({ DEBUG: true, event: "FIN", state }));
67+
}
68+
};
69+
"""
6170

62-
/// Represents a Swift heap object like a class instance or an actor instance.
63-
class SwiftHeapObject {
64-
static __wrap(pointer, deinit, prototype) {
65-
const obj = Object.create(prototype);
66-
const state = { pointer, deinit, hasReleased: false };
67-
obj.pointer = pointer;
68-
obj.__swiftHeapObjectState = state;
69-
swiftHeapObjectFinalizationRegistry.register(obj, state, state);
70-
return obj;
71-
}
72-
73-
release() {
74-
const state = this.__swiftHeapObjectState;
71+
var swiftHeapObjectClassJs: String {
72+
var output = ""
73+
if enableLifetimeTracking {
74+
output += lifetimeTrackingClassJs + "\n"
75+
}
76+
output += """
77+
const swiftHeapObjectFinalizationRegistry = (typeof FinalizationRegistry === "undefined") ? { register: () => {}, unregister: () => {} } : new FinalizationRegistry((state) => {
78+
79+
"""
80+
if enableLifetimeTracking {
81+
output += " TRACKING.finalization(state);\n"
82+
}
83+
output += """
7584
if (state.hasReleased) {
7685
return;
7786
}
7887
state.hasReleased = true;
79-
swiftHeapObjectFinalizationRegistry.unregister(state);
8088
state.deinit(state.pointer);
81-
}
89+
});
90+
91+
/// Represents a Swift heap object like a class instance or an actor instance.
92+
class SwiftHeapObject {
93+
static __wrap(pointer, deinit, prototype) {
94+
const obj = Object.create(prototype);
95+
const state = { pointer, deinit, hasReleased: false };
96+
97+
"""
98+
if enableLifetimeTracking {
99+
output += " TRACKING.wrap(pointer, deinit, prototype, state);\n"
82100
}
83-
"""
101+
output += """
102+
obj.pointer = pointer;
103+
obj.__swiftHeapObjectState = state;
104+
swiftHeapObjectFinalizationRegistry.register(obj, state, state);
105+
return obj;
106+
}
107+
108+
release() {
109+
110+
"""
111+
if enableLifetimeTracking {
112+
output += " TRACKING.release(this);\n"
113+
}
114+
output += """
115+
const state = this.__swiftHeapObjectState;
116+
if (state.hasReleased) {
117+
return;
118+
}
119+
state.hasReleased = true;
120+
swiftHeapObjectFinalizationRegistry.unregister(state);
121+
state.deinit(state.pointer);
122+
}
123+
}
124+
"""
125+
return output
126+
}
84127

85128
fileprivate struct LinkData {
86129
var exportsLines: [String] = []

‎Sources/JavaScriptKit/BridgeJSIntrinsics.swift‎

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ internal func _swift_js_closure_unregister(_ id: Int32) {
124124
// - `func bridgeJSStackPush()`: push the value onto the return stack (used by _BridgedSwiftStackType for array elements)
125125
//
126126
// Optional types (ExportSwift only) additionally define:
127-
// - `func bridgeJSLowerParameterWithRetain()`: lower optional heap object with ownership transfer for escaping closures
128127
// - `func bridgeJSLiftReturnFromSideChannel()`: lift optional from side-channel storage for protocol property getters
129128
//
130129
// See JSGlueGen.swift in BridgeJSLink for JS-side lowering/lifting implementation.
@@ -467,9 +466,10 @@ public protocol _BridgedSwiftHeapObject: AnyObject, _BridgedSwiftStackType {}
467466
extension _BridgedSwiftHeapObject {
468467

469468
// MARK: ImportTS
470-
@_spi(BridgeJS) @_transparent public consuming func bridgeJSLowerParameter() -> UnsafeMutableRawPointer {
471-
// For protocol parameters, we pass the unretained pointer since JS already has a reference
472-
return Unmanaged.passUnretained(self).toOpaque()
469+
@_spi(BridgeJS) @_transparent public func bridgeJSLowerParameter() -> UnsafeMutableRawPointer {
470+
// Transfer ownership to JS for imported SwiftHeapObject parameters.
471+
// JS side must eventually release (via release() or FinalizationRegistry).
472+
return Unmanaged.passRetained(self).toOpaque()
473473
}
474474
@_spi(BridgeJS) @_transparent public static func bridgeJSLiftReturn(_ pointer: UnsafeMutableRawPointer) -> Self {
475475
// For protocol returns, take an unretained value since JS manages the lifetime
@@ -1489,11 +1489,10 @@ extension Optional where Wrapped: _BridgedSwiftHeapObject {
14891489
// MARK: ExportSwift
14901490
/// Lowers optional Swift heap object as (isSome, pointer) tuple for protocol parameters.
14911491
///
1492-
/// This method uses `passUnretained()` because the caller (JavaScript protocol implementation)
1493-
/// already owns the object and will not retain it. The pointer is only valid for the
1494-
/// duration of the call.
1492+
/// Transfer ownership to JavaScript for imported optional heap-object parameters; JS must
1493+
/// release via `release()` or finalizer.
14951494
///
1496-
/// - Returns: A tuple containing presence flag (0 for nil, 1 for some) and unretained pointer
1495+
/// - Returns: A tuple containing presence flag (0 for nil, 1 for some) and retained pointer
14971496
@_spi(BridgeJS) @_transparent public consuming func bridgeJSLowerParameter() -> (
14981497
isSome: Int32, pointer: UnsafeMutableRawPointer
14991498
) {
@@ -1505,25 +1504,6 @@ extension Optional where Wrapped: _BridgedSwiftHeapObject {
15051504
}
15061505
}
15071506

1508-
/// Lowers optional Swift heap object with ownership transfer for escaping closures.
1509-
///
1510-
/// This method uses `passRetained()` to transfer ownership to JavaScript, ensuring the
1511-
/// object remains valid even if the JavaScript closure escapes and stores the parameter.
1512-
/// JavaScript must wrap the pointer with `__construct()` to create a managed reference
1513-
/// that will be cleaned up via FinalizationRegistry.
1514-
///
1515-
/// - Returns: A tuple containing presence flag (0 for nil, 1 for some) and retained pointer
1516-
@_spi(BridgeJS) @_transparent public consuming func bridgeJSLowerParameterWithRetain() -> (
1517-
isSome: Int32, pointer: UnsafeMutableRawPointer
1518-
) {
1519-
switch consume self {
1520-
case .none:
1521-
return (isSome: 0, pointer: UnsafeMutableRawPointer(bitPattern: 1)!)
1522-
case .some(let value):
1523-
return (isSome: 1, pointer: Unmanaged.passRetained(value).toOpaque())
1524-
}
1525-
}
1526-
15271507
@_spi(BridgeJS) @_transparent public static func bridgeJSLiftReturn(_ pointer: UnsafeMutableRawPointer) -> Wrapped?
15281508
{
15291509
if pointer == UnsafeMutableRawPointer(bitPattern: 0) {
@@ -2008,12 +1988,6 @@ extension _BridgedAsOptional where Wrapped: _BridgedSwiftHeapObject {
20081988
asOptional.bridgeJSLowerParameter()
20091989
}
20101990

2011-
@_spi(BridgeJS) public consuming func bridgeJSLowerParameterWithRetain() -> (
2012-
isSome: Int32, pointer: UnsafeMutableRawPointer
2013-
) {
2014-
asOptional.bridgeJSLowerParameterWithRetain()
2015-
}
2016-
20171991
@_spi(BridgeJS) public static func bridgeJSLiftParameter(
20181992
_ isSome: Int32,
20191993
_ pointer: UnsafeMutableRawPointer

‎Tests/BridgeJSRuntimeTests/Generated/BridgeJS.swift‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9190,6 +9190,45 @@ fileprivate func _bjs_Container_wrap_extern(_ pointer: UnsafeMutableRawPointer)
91909190
return _bjs_Container_wrap_extern(pointer)
91919191
}
91929192

9193+
@_expose(wasm, "bjs_LeakCheck_init")
9194+
@_cdecl("bjs_LeakCheck_init")
9195+
public func _bjs_LeakCheck_init() -> UnsafeMutableRawPointer {
9196+
#if arch(wasm32)
9197+
let ret = LeakCheck()
9198+
return ret.bridgeJSLowerReturn()
9199+
#else
9200+
fatalError("Only available on WebAssembly")
9201+
#endif
9202+
}
9203+
9204+
@_expose(wasm, "bjs_LeakCheck_deinit")
9205+
@_cdecl("bjs_LeakCheck_deinit")
9206+
public func _bjs_LeakCheck_deinit(_ pointer: UnsafeMutableRawPointer) -> Void {
9207+
#if arch(wasm32)
9208+
Unmanaged<LeakCheck>.fromOpaque(pointer).release()
9209+
#else
9210+
fatalError("Only available on WebAssembly")
9211+
#endif
9212+
}
9213+
9214+
extension LeakCheck: ConvertibleToJSValue, _BridgedSwiftHeapObject {
9215+
public var jsValue: JSValue {
9216+
return .object(JSObject(id: UInt32(bitPattern: _bjs_LeakCheck_wrap(Unmanaged.passRetained(self).toOpaque()))))
9217+
}
9218+
}
9219+
9220+
#if arch(wasm32)
9221+
@_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_LeakCheck_wrap")
9222+
fileprivate func _bjs_LeakCheck_wrap_extern(_ pointer: UnsafeMutableRawPointer) -> Int32
9223+
#else
9224+
fileprivate func _bjs_LeakCheck_wrap_extern(_ pointer: UnsafeMutableRawPointer) -> Int32 {
9225+
fatalError("Only available on WebAssembly")
9226+
}
9227+
#endif
9228+
@inline(never) fileprivate func _bjs_LeakCheck_wrap(_ pointer: UnsafeMutableRawPointer) -> Int32 {
9229+
return _bjs_LeakCheck_wrap_extern(pointer)
9230+
}
9231+
91939232
#if arch(wasm32)
91949233
@_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_ClosureSupportImports_jsApplyVoid_static")
91959234
fileprivate func bjs_ClosureSupportImports_jsApplyVoid_static_extern(_ callback: Int32) -> Void
@@ -11252,6 +11291,30 @@ fileprivate func bjs_SwiftClassSupportImports_jsRoundTripOptionalGreeter_static_
1125211291
return bjs_SwiftClassSupportImports_jsRoundTripOptionalGreeter_static_extern(greeterIsSome, greeterPointer)
1125311292
}
1125411293

11294+
#if arch(wasm32)
11295+
@_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static")
11296+
fileprivate func bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static_extern(_ value: UnsafeMutableRawPointer) -> Void
11297+
#else
11298+
fileprivate func bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static_extern(_ value: UnsafeMutableRawPointer) -> Void {
11299+
fatalError("Only available on WebAssembly")
11300+
}
11301+
#endif
11302+
@inline(never) fileprivate func bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static(_ value: UnsafeMutableRawPointer) -> Void {
11303+
return bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static_extern(value)
11304+
}
11305+
11306+
#if arch(wasm32)
11307+
@_extern(wasm, module: "BridgeJSRuntimeTests", name: "bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static")
11308+
fileprivate func bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static_extern(_ valueIsSome: Int32, _ valuePointer: UnsafeMutableRawPointer) -> Void
11309+
#else
11310+
fileprivate func bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static_extern(_ valueIsSome: Int32, _ valuePointer: UnsafeMutableRawPointer) -> Void {
11311+
fatalError("Only available on WebAssembly")
11312+
}
11313+
#endif
11314+
@inline(never) fileprivate func bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static(_ valueIsSome: Int32, _ valuePointer: UnsafeMutableRawPointer) -> Void {
11315+
return bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static_extern(valueIsSome, valuePointer)
11316+
}
11317+
1125511318
func _$SwiftClassSupportImports_jsRoundTripGreeter(_ greeter: Greeter) throws(JSException) -> Greeter {
1125611319
let greeterPointer = greeter.bridgeJSLowerParameter()
1125711320
let ret = bjs_SwiftClassSupportImports_jsRoundTripGreeter_static(greeterPointer)
@@ -11268,4 +11331,20 @@ func _$SwiftClassSupportImports_jsRoundTripOptionalGreeter(_ greeter: Optional<G
1126811331
throw error
1126911332
}
1127011333
return Optional<Greeter>.bridgeJSLiftReturn(ret)
11334+
}
11335+
11336+
func _$SwiftClassSupportImports_jsConsumeLeakCheck(_ value: LeakCheck) throws(JSException) -> Void {
11337+
let valuePointer = value.bridgeJSLowerParameter()
11338+
bjs_SwiftClassSupportImports_jsConsumeLeakCheck_static(valuePointer)
11339+
if let error = _swift_js_take_exception() {
11340+
throw error
11341+
}
11342+
}
11343+
11344+
func _$SwiftClassSupportImports_jsConsumeOptionalLeakCheck(_ value: Optional<LeakCheck>) throws(JSException) -> Void {
11345+
let (valueIsSome, valuePointer) = value.bridgeJSLowerParameter()
11346+
bjs_SwiftClassSupportImports_jsConsumeOptionalLeakCheck_static(valueIsSome, valuePointer)
11347+
if let error = _swift_js_take_exception() {
11348+
throw error
11349+
}
1127111350
}

‎Tests/BridgeJSRuntimeTests/Generated/JavaScript/BridgeJS.json‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3626,6 +3626,28 @@
36263626
}
36273627
],
36283628
"swiftCallName" : "Container"
3629+
},
3630+
{
3631+
"constructor" : {
3632+
"abiName" : "bjs_LeakCheck_init",
3633+
"effects" : {
3634+
"isAsync" : false,
3635+
"isStatic" : false,
3636+
"isThrows" : false
3637+
},
3638+
"parameters" : [
3639+
3640+
]
3641+
},
3642+
"explicitAccessControl" : "public",
3643+
"methods" : [
3644+
3645+
],
3646+
"name" : "LeakCheck",
3647+
"properties" : [
3648+
3649+
],
3650+
"swiftCallName" : "LeakCheck"
36293651
}
36303652
],
36313653
"enums" : [
@@ -15643,6 +15665,47 @@
1564315665
"_1" : "null"
1564415666
}
1564515667
}
15668+
},
15669+
{
15670+
"name" : "jsConsumeLeakCheck",
15671+
"parameters" : [
15672+
{
15673+
"name" : "value",
15674+
"type" : {
15675+
"swiftHeapObject" : {
15676+
"_0" : "LeakCheck"
15677+
}
15678+
}
15679+
}
15680+
],
15681+
"returnType" : {
15682+
"void" : {
15683+
15684+
}
15685+
}
15686+
},
15687+
{
15688+
"name" : "jsConsumeOptionalLeakCheck",
15689+
"parameters" : [
15690+
{
15691+
"name" : "value",
15692+
"type" : {
15693+
"nullable" : {
15694+
"_0" : {
15695+
"swiftHeapObject" : {
15696+
"_0" : "LeakCheck"
15697+
}
15698+
},
15699+
"_1" : "null"
15700+
}
15701+
}
15702+
}
15703+
],
15704+
"returnType" : {
15705+
"void" : {
15706+
15707+
}
15708+
}
1564615709
}
1564715710
]
1564815711
}

‎Tests/BridgeJSRuntimeTests/JavaScript/SwiftClassSupportTests.mjs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,14 @@ export function getImports(importsContext) {
99
jsRoundTripOptionalGreeter: (greeter) => {
1010
return greeter;
1111
},
12+
jsConsumeLeakCheck: (value) => {
13+
// Explicitly release on JS side to mimic user cleanup.
14+
value.release();
15+
},
16+
jsConsumeOptionalLeakCheck: (value) => {
17+
if (value) {
18+
value.release();
19+
}
20+
},
1221
};
13-
}
22+
}

0 commit comments

Comments
 (0)