From 4cd8c16e7b3bf08e1b654e504eeeca035ff1ca9d Mon Sep 17 00:00:00 2001 From: Iceman Date: Wed, 20 May 2026 12:24:27 +0900 Subject: [PATCH 1/2] Add generic createDestroyFunction impl and add double free check --- ...t2JavaGenerator+JavaBindingsPrinting.swift | 54 ------------------- Sources/SwiftJava/SwiftObjects.swift | 9 ++++ .../swift/swiftkit/core/JNISwiftInstance.java | 33 ++++++++++-- .../core/JNISwiftInstanceCleanup.java | 15 ++++-- .../org/swift/swiftkit/core/SwiftObjects.java | 1 + .../swiftkit/ffm/FFMSwiftErrorInstance.java | 5 +- .../swift/swiftkit/ffm/FFMSwiftInstance.java | 5 +- .../swiftkit/ffm/FFMSwiftInstanceCleanup.java | 21 ++++---- 8 files changed, 62 insertions(+), 81 deletions(-) diff --git a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift index 330a2191..0b0fb8dc 100644 --- a/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift +++ b/Sources/JExtractSwiftLib/JNI/JNISwift2JavaGenerator+JavaBindingsPrinting.swift @@ -383,8 +383,6 @@ extension JNISwift2JavaGenerator { """ ) printer.println() - - printDestroyFunction(&printer, decl) } } @@ -876,58 +874,6 @@ extension JNISwift2JavaGenerator { } } - /// Prints the destroy function for a `JNISwiftInstance` - private func printDestroyFunction(_ printer: inout CodePrinter, _ type: ImportedNominalType) { - let funcName = "$createDestroyFunction" - let isEffectivelyGeneric = type.swiftNominal.isGeneric && !type.isSpecialization - let typeName = type.effectiveJavaSimpleName - printer.print("@Override") - printer.printBraceBlock("public Runnable \(funcName)()") { printer in - printer.print("long self$ = this.$memoryAddress();") - printer.print("long selfType$ = this.$typeMetadataAddress();") - if isEffectivelyGeneric { - printer.print( - """ - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("\(typeName).\(funcName)", - "this", this, - "self", self$, - "selfType", selfType$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("\(typeName).$destroy", "self", self$, "selfType", selfType$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - """ - ) - } else { - printer.print( - """ - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("\(typeName).\(funcName)", - "this", this, - "self", self$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("\(typeName).$destroy", "self", self$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - """ - ) - } - } - } - private func printFoundationDateHelpers(_ printer: inout CodePrinter, _ decl: ImportedNominalType) { printer.print( """ diff --git a/Sources/SwiftJava/SwiftObjects.swift b/Sources/SwiftJava/SwiftObjects.swift index 5df76c94..cf5e02c9 100644 --- a/Sources/SwiftJava/SwiftObjects.swift +++ b/Sources/SwiftJava/SwiftObjects.swift @@ -89,4 +89,13 @@ extension SwiftObjects { } return perform(as: typeMetadata) } + + @JavaMethod + public static func typeDescription(environment: UnsafeMutablePointer!, selfTypePointer: Int64) -> String { + guard let selfType$ = UnsafeRawPointer(bitPattern: Int(selfTypePointer)) else { + fatalError("selfType metadata address was null") + } + let typeMetadata = unsafeBitCast(selfType$, to: Any.Type.self) + return String(describing: typeMetadata) + } } diff --git a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstance.java b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstance.java index 4e3b8378..01734d75 100644 --- a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstance.java +++ b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstance.java @@ -27,15 +27,38 @@ public interface JNISwiftInstance extends SwiftInstance { * * @return a function that is called when the value should be destroyed. */ - Runnable $createDestroyFunction(); + default Runnable $createDestroyFunction() { + var memoryAddress = $memoryAddress(); + var typeMetadataAddress = $typeMetadataAddress(); + if (CallTraces.TRACE_DOWNCALLS) { + CallTraces.trace( + "this", this, + "self", memoryAddress, + "selfType", SwiftObjects.typeDescription(typeMetadataAddress) + ); + } + return () -> { + if (CallTraces.TRACE_DOWNCALLS) { + CallTraces.traceDowncall( + "SwiftObjects.destroy", + "self", memoryAddress, + "selfType", SwiftObjects.typeDescription(typeMetadataAddress) + ); + } + SwiftObjects.destroy(memoryAddress, typeMetadataAddress); + }; + } + /** + * The Swift type metadata address of this type. + */ long $typeMetadataAddress(); @Override default SwiftInstanceCleanup $createCleanup() { - var statusDestroyedFlag = $statusDestroyedFlag(); - Runnable markAsDestroyed = () -> statusDestroyedFlag.set(true); - - return new JNISwiftInstanceCleanup(this.$createDestroyFunction(), markAsDestroyed); + return new JNISwiftInstanceCleanup( + $createDestroyFunction(), + $statusDestroyedFlag() + ); } } diff --git a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstanceCleanup.java b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstanceCleanup.java index 08ddcdab..1dce368c 100644 --- a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstanceCleanup.java +++ b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/JNISwiftInstanceCleanup.java @@ -14,18 +14,23 @@ package org.swift.swiftkit.core; +import java.util.concurrent.atomic.AtomicBoolean; + class JNISwiftInstanceCleanup implements SwiftInstanceCleanup { private final Runnable destroyFunction; - private final Runnable markAsDestroyed; + private final AtomicBoolean statusDestroyedFlag; - public JNISwiftInstanceCleanup(Runnable destroyFunction, Runnable markAsDestroyed) { + public JNISwiftInstanceCleanup(Runnable destroyFunction, AtomicBoolean statusDestroyedFlag) { this.destroyFunction = destroyFunction; - this.markAsDestroyed = markAsDestroyed; + this.statusDestroyedFlag = statusDestroyedFlag; } @Override public void run() { - markAsDestroyed.run(); - destroyFunction.run(); + if (statusDestroyedFlag.compareAndSet(false, true)) { + destroyFunction.run(); + } else { + throw new IllegalStateException("Double destruction attempt detected!"); + } } } diff --git a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftObjects.java b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftObjects.java index 3674198e..d98f722d 100644 --- a/SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftObjects.java +++ b/SwiftKitCore/src/main/java/org/swift/swiftkit/core/SwiftObjects.java @@ -28,4 +28,5 @@ public static void requireNonZero(long number, String name) { public static native String toString(long selfPointer, long selfTypePointer); public static native String toDebugString(long selfPointer, long selfTypePointer); public static native void destroy(long selfPointer, long selfTypePointer); + public static native String typeDescription(long selfTypePointer); } diff --git a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftErrorInstance.java b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftErrorInstance.java index 350337c2..224faaec 100644 --- a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftErrorInstance.java +++ b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftErrorInstance.java @@ -61,13 +61,10 @@ protected FFMSwiftErrorInstance(MemorySegment segment, AllocatingSwiftArena aren @Override public SwiftInstanceCleanup $createCleanup() { - var statusDestroyedFlag = $statusDestroyedFlag(); - Runnable markAsDestroyed = () -> statusDestroyedFlag.set(true); - return new FFMSwiftInstanceCleanup( $memorySegment(), $swiftType(), - markAsDestroyed + $statusDestroyedFlag() ); } } diff --git a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstance.java b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstance.java index c17c8ca1..bc8a2d9e 100644 --- a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstance.java +++ b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstance.java @@ -70,13 +70,10 @@ protected FFMSwiftInstance(MemorySegment segment, AllocatingSwiftArena arena) { @Override public SwiftInstanceCleanup $createCleanup() { - var statusDestroyedFlag = $statusDestroyedFlag(); - Runnable markAsDestroyed = () -> statusDestroyedFlag.set(true); - return new FFMSwiftInstanceCleanup( $memorySegment(), $swiftType(), - markAsDestroyed + $statusDestroyedFlag() ); } diff --git a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstanceCleanup.java b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstanceCleanup.java index 7b7bb1ec..97c4808d 100644 --- a/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstanceCleanup.java +++ b/SwiftKitFFM/src/main/java/org/swift/swiftkit/ffm/FFMSwiftInstanceCleanup.java @@ -19,26 +19,29 @@ import static org.swift.swiftkit.ffm.SwiftJavaLogGroup.LIFECYCLE; import java.lang.foreign.MemorySegment; +import java.util.concurrent.atomic.AtomicBoolean; public class FFMSwiftInstanceCleanup implements SwiftInstanceCleanup { private final MemorySegment memoryAddress; private final SwiftAnyType type; - private final Runnable markAsDestroyed; + private final AtomicBoolean statusDestroyedFlag; - public FFMSwiftInstanceCleanup(MemorySegment memoryAddress, SwiftAnyType type, Runnable markAsDestroyed) { + public FFMSwiftInstanceCleanup(MemorySegment memoryAddress, SwiftAnyType type, AtomicBoolean statusDestroyedFlag) { this.memoryAddress = memoryAddress; this.type = type; - this.markAsDestroyed = markAsDestroyed; + this.statusDestroyedFlag = statusDestroyedFlag; } @Override public void run() { - markAsDestroyed.run(); - - // Allow null pointers just for AutoArena tests. - if (type != null && memoryAddress != null) { - SwiftRuntime.log(LIFECYCLE, "Destroy swift value [" + type.getSwiftName() + "]: " + memoryAddress); - SwiftValueWitnessTable.destroy(type, memoryAddress); + if (statusDestroyedFlag.compareAndSet(false, true)) { + // Allow null pointers just for AutoArena tests. + if (type != null && memoryAddress != null) { + SwiftRuntime.log(LIFECYCLE, "Destroy swift value [" + type.getSwiftName() + "]: " + memoryAddress); + SwiftValueWitnessTable.destroy(type, memoryAddress); + } + } else { + throw new IllegalStateException("Double destruction attempt detected!"); } } } From ca07baf6e50762d8b652308455805b39634c4d7d Mon Sep 17 00:00:00 2001 From: Iceman Date: Wed, 20 May 2026 12:34:54 +0900 Subject: [PATCH 2/2] Fix tests --- .../JNI/JNIClassTests.swift | 27 ------------------ .../JExtractSwiftTests/JNI/JNIEnumTests.swift | 21 -------------- .../JNI/JNIGenericTypeTests.swift | 22 --------------- .../JNI/JNIStructTests.swift | 28 ------------------- 4 files changed, 98 deletions(-) diff --git a/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift b/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift index efa1a966..00d2b4bd 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIClassTests.swift @@ -97,33 +97,6 @@ struct JNIClassTests { """, ] ) - try assertOutput( - input: source, - .jni, - .java, - expectedChunks: [ - """ - @Override - public Runnable $createDestroyFunction() { - long self$ = this.$memoryAddress(); - long selfType$ = this.$typeMetadataAddress(); - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyClass.$createDestroyFunction", - "this", this, - "self", self$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyClass.$destroy", "self", self$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - """ - ] - ) } @Test diff --git a/Tests/JExtractSwiftTests/JNI/JNIEnumTests.swift b/Tests/JExtractSwiftTests/JNI/JNIEnumTests.swift index 3c4a41ad..8c7a3714 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIEnumTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIEnumTests.swift @@ -83,27 +83,6 @@ struct JNIEnumTests { return new MyEnum(selfPointer, swiftArena); } """, - """ - @Override - public Runnable $createDestroyFunction() { - long self$ = this.$memoryAddress(); - long selfType$ = this.$typeMetadataAddress(); - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyEnum.$createDestroyFunction", - "this", this, - "self", self$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyEnum.$destroy", "self", self$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - } - """, ] ) } diff --git a/Tests/JExtractSwiftTests/JNI/JNIGenericTypeTests.swift b/Tests/JExtractSwiftTests/JNI/JNIGenericTypeTests.swift index 3760c710..7570580a 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIGenericTypeTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIGenericTypeTests.swift @@ -76,28 +76,6 @@ struct JNIGenericTypeTests { return this.selfTypePointer; } """, - """ - @Override - public Runnable $createDestroyFunction() { - long self$ = this.$memoryAddress(); - long selfType$ = this.$typeMetadataAddress(); - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyID.$createDestroyFunction", - "this", this, - "self", self$, - "selfType", selfType$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyID.$destroy", "self", self$, "selfType", selfType$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - } - """, ] ) } diff --git a/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift b/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift index 2147079d..8739a96a 100644 --- a/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift +++ b/Tests/JExtractSwiftTests/JNI/JNIStructTests.swift @@ -83,34 +83,6 @@ struct JNIStructTests { """, ] ) - try assertOutput( - input: source, - .jni, - .java, - expectedChunks: [ - """ - @Override - public Runnable $createDestroyFunction() { - long self$ = this.$memoryAddress(); - long selfType$ = this.$typeMetadataAddress(); - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyStruct.$createDestroyFunction", - "this", this, - "self", self$); - } - return new Runnable() { - @Override - public void run() { - if (CallTraces.TRACE_DOWNCALLS) { - CallTraces.traceDowncall("MyStruct.$destroy", "self", self$); - } - SwiftObjects.destroy(self$, selfType$); - } - }; - } - """ - ] - ) } @Test