Skip to content

Commit f1d5c94

Browse files
krodakkrodak
committed
BridgeJS: Fix JSValue reference ownership across the bridge
1 parent 0d41cbf commit f1d5c94

28 files changed

Lines changed: 330 additions & 95 deletions

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

Lines changed: 64 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,27 @@ struct IntrinsicJSFragment: Sendable {
749749

750750
private static let jsValueLowerHelperName = "__bjs_jsValueLower"
751751
private static let jsValueLiftHelperName = "__bjs_jsValueLift"
752+
private static let jsValueTakeHelperName = "__bjs_jsValueTake"
753+
754+
/// Mirrors `JavaScriptValueKind` in `_CJavaScriptKit.h`.
755+
private enum JSValueKind: Int, CaseIterable {
756+
case boolean = 0
757+
case string = 1
758+
case number = 2
759+
case object = 3
760+
case null = 4
761+
case undefined = 5
762+
case symbol = 7
763+
case bigInt = 8
764+
765+
/// Whether `payload1` is a reference in `swift.memory`.
766+
var holdsReference: Bool {
767+
switch self {
768+
case .string, .object, .symbol, .bigInt: return true
769+
case .boolean, .number, .null, .undefined: return false
770+
}
771+
}
772+
}
752773

753774
private static func registerJSValueHelpers(scope: JSGlueVariableScope) {
754775
scope.registerIntrinsic("jsValueHelpers") { helperPrinter in
@@ -777,6 +798,20 @@ struct IntrinsicJSFragment: Sendable {
777798
helperPrinter.write("return \(resultVar);")
778799
}
779800
helperPrinter.write("}")
801+
helperPrinter.write("function \(jsValueTakeHelperName)(kind, payload1, payload2) {")
802+
helperPrinter.indent {
803+
helperPrinter.write("const value = \(jsValueLiftHelperName)(kind, payload1, payload2);")
804+
let holdsReference = JSValueKind.allCases.filter(\.holdsReference)
805+
.map { "kind === \($0.rawValue)" }
806+
.joined(separator: " || ")
807+
helperPrinter.write("if (\(holdsReference)) {")
808+
helperPrinter.indent {
809+
helperPrinter.write("\(JSGlueVariableScope.reservedSwift).memory.release(payload1);")
810+
}
811+
helperPrinter.write("}")
812+
helperPrinter.write("return value;")
813+
}
814+
helperPrinter.write("}")
780815
}
781816
}
782817

@@ -792,7 +827,7 @@ struct IntrinsicJSFragment: Sendable {
792827
printer.write("let \(payload2Var);")
793828
printer.write("if (\(value) === null) {")
794829
printer.indent {
795-
printer.write("\(kindVar) = 4;")
830+
printer.write("\(kindVar) = \(JSValueKind.null.rawValue);")
796831
printer.write("\(payload1Var) = 0;")
797832
printer.write("\(payload2Var) = 0;")
798833
}
@@ -802,41 +837,41 @@ struct IntrinsicJSFragment: Sendable {
802837
printer.indent {
803838
printer.write("case \"boolean\":")
804839
printer.indent {
805-
printer.write("\(kindVar) = 0;")
840+
printer.write("\(kindVar) = \(JSValueKind.boolean.rawValue);")
806841
printer.write("\(payload1Var) = \(value) ? 1 : 0;")
807842
printer.write("\(payload2Var) = 0;")
808843
printer.write("break;")
809844
}
810845
printer.write("case \"number\":")
811846
printer.indent {
812-
printer.write("\(kindVar) = 2;")
847+
printer.write("\(kindVar) = \(JSValueKind.number.rawValue);")
813848
printer.write("\(payload1Var) = 0;")
814849
printer.write("\(payload2Var) = \(value);")
815850
printer.write("break;")
816851
}
817-
func emitRetainCase(_ caseName: String, kind: Int) {
852+
func emitRetainCase(_ caseName: String, kind: JSValueKind) {
818853
printer.write("case \"\(caseName)\":")
819854
printer.indent {
820-
printer.write("\(kindVar) = \(kind);")
855+
printer.write("\(kindVar) = \(kind.rawValue);")
821856
printer.write(
822857
"\(payload1Var) = \(JSGlueVariableScope.reservedSwift).memory.retain(\(value));"
823858
)
824859
printer.write("\(payload2Var) = 0;")
825860
printer.write("break;")
826861
}
827862
}
828-
emitRetainCase("string", kind: 1)
863+
emitRetainCase("string", kind: .string)
829864
printer.write("case \"undefined\":")
830865
printer.indent {
831-
printer.write("\(kindVar) = 5;")
866+
printer.write("\(kindVar) = \(JSValueKind.undefined.rawValue);")
832867
printer.write("\(payload1Var) = 0;")
833868
printer.write("\(payload2Var) = 0;")
834869
printer.write("break;")
835870
}
836-
emitRetainCase("object", kind: 3)
837-
emitRetainCase("function", kind: 3)
838-
emitRetainCase("symbol", kind: 7)
839-
emitRetainCase("bigint", kind: 8)
871+
emitRetainCase("object", kind: .object)
872+
emitRetainCase("function", kind: .object)
873+
emitRetainCase("symbol", kind: .symbol)
874+
emitRetainCase("bigint", kind: .bigInt)
840875
printer.write("default:")
841876
printer.indent {
842877
printer.write("throw new TypeError(\"Unsupported JSValue type\");")
@@ -874,53 +909,21 @@ struct IntrinsicJSFragment: Sendable {
874909
printer.write("let \(resultVar);")
875910
printer.write("switch (\(kind)) {")
876911
printer.indent {
877-
printer.write("case 0:")
878-
printer.indent {
879-
printer.write("\(resultVar) = \(payload1) !== 0;")
880-
printer.write("break;")
881-
}
882-
printer.write("case 1:")
883-
printer.indent {
884-
printer.write(
885-
"\(resultVar) = \(JSGlueVariableScope.reservedSwift).memory.getObject(\(payload1));"
886-
)
887-
printer.write("break;")
888-
}
889-
printer.write("case 2:")
890-
printer.indent {
891-
printer.write("\(resultVar) = \(payload2);")
892-
printer.write("break;")
893-
}
894-
printer.write("case 3:")
895-
printer.indent {
896-
printer.write(
897-
"\(resultVar) = \(JSGlueVariableScope.reservedSwift).memory.getObject(\(payload1));"
898-
)
899-
printer.write("break;")
900-
}
901-
printer.write("case 4:")
902-
printer.indent {
903-
printer.write("\(resultVar) = null;")
904-
printer.write("break;")
905-
}
906-
printer.write("case 5:")
907-
printer.indent {
908-
printer.write("\(resultVar) = undefined;")
909-
printer.write("break;")
910-
}
911-
printer.write("case 7:")
912-
printer.indent {
913-
printer.write(
914-
"\(resultVar) = \(JSGlueVariableScope.reservedSwift).memory.getObject(\(payload1));"
915-
)
916-
printer.write("break;")
917-
}
918-
printer.write("case 8:")
919-
printer.indent {
920-
printer.write(
921-
"\(resultVar) = \(JSGlueVariableScope.reservedSwift).memory.getObject(\(payload1));"
922-
)
923-
printer.write("break;")
912+
for valueKind in JSValueKind.allCases {
913+
let value: String
914+
switch valueKind {
915+
case .boolean: value = "\(payload1) !== 0"
916+
case .number: value = payload2
917+
case .null: value = "null"
918+
case .undefined: value = "undefined"
919+
case .string, .object, .symbol, .bigInt:
920+
value = "\(JSGlueVariableScope.reservedSwift).memory.getObject(\(payload1))"
921+
}
922+
printer.write("case \(valueKind.rawValue):")
923+
printer.indent {
924+
printer.write("\(resultVar) = \(value);")
925+
printer.write("break;")
926+
}
924927
}
925928
printer.write("default:")
926929
printer.indent {
@@ -961,11 +964,12 @@ struct IntrinsicJSFragment: Sendable {
961964
let resultVar = scope.variable("jsValue")
962965
registerJSValueHelpers(scope: scope)
963966
printer.write(
964-
"const \(resultVar) = \(jsValueLiftHelperName)(\(kind), \(payload1), \(payload2));"
967+
"const \(resultVar) = \(jsValueTakeHelperName)(\(kind), \(payload1), \(payload2));"
965968
)
966969
return [resultVar]
967970
}
968971
)
972+
/// Swift lends the payload reference for the duration of the call, so it is not released here.
969973
static let jsValueLiftParameter = IntrinsicJSFragment(
970974
parameters: ["kind", "payload1", "payload2"],
971975
printCode: { arguments, context in
@@ -2232,7 +2236,7 @@ struct IntrinsicJSFragment: Sendable {
22322236
let resultVar = scope.variable("jsValue")
22332237
registerJSValueHelpers(scope: scope)
22342238
printer.write(
2235-
"const \(resultVar) = \(jsValueLiftHelperName)(\(kindVar), \(payload1Var), \(payload2Var));"
2239+
"const \(resultVar) = \(jsValueTakeHelperName)(\(kindVar), \(payload1Var), \(payload2Var));"
22362240
)
22372241
return [resultVar]
22382242
}

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Alias.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ export async function createInstantiator(options, swift) {
267267
const jsValuePayload2 = f64Stack.pop();
268268
const jsValuePayload1 = i32Stack.pop();
269269
const jsValueKind = i32Stack.pop();
270-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
270+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
271271
return jsValue;
272272
},
273273
},
@@ -361,6 +361,13 @@ export async function createInstantiator(options, swift) {
361361
}
362362
return jsValue;
363363
}
364+
function __bjs_jsValueTake(kind, payload1, payload2) {
365+
const value = __bjs_jsValueLift(kind, payload1, payload2);
366+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
367+
swift.memory.release(payload1);
368+
}
369+
return value;
370+
}
364371

365372
const __bjs_codec_M10TestModuleT16PolygonReference = {
366373
lower: (v) => {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/ArrayTypes.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ export async function createInstantiator(options, swift) {
274274
const jsValuePayload2 = f64Stack.pop();
275275
const jsValuePayload1 = i32Stack.pop();
276276
const jsValueKind = i32Stack.pop();
277-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
277+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
278278
return jsValue;
279279
},
280280
},
@@ -368,6 +368,13 @@ export async function createInstantiator(options, swift) {
368368
}
369369
return jsValue;
370370
}
371+
function __bjs_jsValueTake(kind, payload1, payload2) {
372+
const value = __bjs_jsValueLift(kind, payload1, payload2);
373+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
374+
swift.memory.release(payload1);
375+
}
376+
return value;
377+
}
371378

372379
const __bjs_codec_Array_Int = __bjs_arrayCodec(__bjs_primitiveCodecs.Int);
373380
const __bjs_codec_Array_String = __bjs_arrayCodec(__bjs_stringCodec);

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/Async.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ export async function createInstantiator(options, swift) {
271271
const jsValuePayload2 = f64Stack.pop();
272272
const jsValuePayload1 = i32Stack.pop();
273273
const jsValueKind = i32Stack.pop();
274-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
274+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
275275
return jsValue;
276276
},
277277
},
@@ -365,6 +365,13 @@ export async function createInstantiator(options, swift) {
365365
}
366366
return jsValue;
367367
}
368+
function __bjs_jsValueTake(kind, payload1, payload2) {
369+
const value = __bjs_jsValueLift(kind, payload1, payload2);
370+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
371+
swift.memory.release(payload1);
372+
}
373+
return value;
374+
}
368375

369376
const __bjs_codec_M10TestModuleT10AsyncPoint = {
370377
lower: (v) => {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/AsyncAssociatedValueEnum.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,13 @@ export async function createInstantiator(options, swift) {
128128
}
129129
return jsValue;
130130
}
131+
function __bjs_jsValueTake(kind, payload1, payload2) {
132+
const value = __bjs_jsValueLift(kind, payload1, payload2);
133+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
134+
swift.memory.release(payload1);
135+
}
136+
return value;
137+
}
131138

132139
const __bjs_createEnumHelpers_M10TestModuleT18AsyncPayloadResult = () => ({
133140
lower: (value) => {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/AsyncImport.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ export async function createInstantiator(options, swift) {
121121
}
122122
return jsValue;
123123
}
124+
function __bjs_jsValueTake(kind, payload1, payload2) {
125+
const value = __bjs_jsValueLift(kind, payload1, payload2);
126+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
127+
swift.memory.release(payload1);
128+
}
129+
return value;
130+
}
124131

125132
const swiftClosureRegistry = (typeof FinalizationRegistry === "undefined") ? { register: () => {}, unregister: () => {} } : new FinalizationRegistry((state) => {
126133
if (state.unregistered) { return; }

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/AsyncStaticImport.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,13 @@ export async function createInstantiator(options, swift) {
121121
}
122122
return jsValue;
123123
}
124+
function __bjs_jsValueTake(kind, payload1, payload2) {
125+
const value = __bjs_jsValueLift(kind, payload1, payload2);
126+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
127+
swift.memory.release(payload1);
128+
}
129+
return value;
130+
}
124131

125132
const swiftClosureRegistry = (typeof FinalizationRegistry === "undefined") ? { register: () => {}, unregister: () => {} } : new FinalizationRegistry((state) => {
126133
if (state.unregistered) { return; }

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/DefaultParameters.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ export async function createInstantiator(options, swift) {
267267
const jsValuePayload2 = f64Stack.pop();
268268
const jsValuePayload1 = i32Stack.pop();
269269
const jsValueKind = i32Stack.pop();
270-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
270+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
271271
return jsValue;
272272
},
273273
},
@@ -361,6 +361,13 @@ export async function createInstantiator(options, swift) {
361361
}
362362
return jsValue;
363363
}
364+
function __bjs_jsValueTake(kind, payload1, payload2) {
365+
const value = __bjs_jsValueLift(kind, payload1, payload2);
366+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
367+
swift.memory.release(payload1);
368+
}
369+
return value;
370+
}
364371

365372
const __bjs_codec_M10TestModuleT6Config = {
366373
lower: (v) => {

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/DictionaryTypes.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ export async function createInstantiator(options, swift) {
261261
const jsValuePayload2 = f64Stack.pop();
262262
const jsValuePayload1 = i32Stack.pop();
263263
const jsValueKind = i32Stack.pop();
264-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
264+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
265265
return jsValue;
266266
},
267267
},
@@ -355,6 +355,13 @@ export async function createInstantiator(options, swift) {
355355
}
356356
return jsValue;
357357
}
358+
function __bjs_jsValueTake(kind, payload1, payload2) {
359+
const value = __bjs_jsValueLift(kind, payload1, payload2);
360+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
361+
swift.memory.release(payload1);
362+
}
363+
return value;
364+
}
358365

359366
const __bjs_codec_Dict_Int = __bjs_dictCodec(__bjs_primitiveCodecs.Int);
360367
const __bjs_codec_Dict_String = __bjs_dictCodec(__bjs_stringCodec);

‎Plugins/BridgeJS/Tests/BridgeJSToolTests/__Snapshots__/BridgeJSLinkTests/EnumAssociatedValue.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ export async function createInstantiator(options, swift) {
342342
const jsValuePayload2 = f64Stack.pop();
343343
const jsValuePayload1 = i32Stack.pop();
344344
const jsValueKind = i32Stack.pop();
345-
const jsValue = __bjs_jsValueLift(jsValueKind, jsValuePayload1, jsValuePayload2);
345+
const jsValue = __bjs_jsValueTake(jsValueKind, jsValuePayload1, jsValuePayload2);
346346
return jsValue;
347347
},
348348
},
@@ -436,6 +436,13 @@ export async function createInstantiator(options, swift) {
436436
}
437437
return jsValue;
438438
}
439+
function __bjs_jsValueTake(kind, payload1, payload2) {
440+
const value = __bjs_jsValueLift(kind, payload1, payload2);
441+
if (kind === 1 || kind === 3 || kind === 7 || kind === 8) {
442+
swift.memory.release(payload1);
443+
}
444+
return value;
445+
}
439446

440447
const __bjs_codec_Optional_String = __bjs_optionalCodec(__bjs_stringCodec);
441448
const __bjs_codec_Optional_Bool = __bjs_optionalCodec(__bjs_primitiveCodecs.Bool);

0 commit comments

Comments
 (0)