avoid capturing instance in cleanup actions

Author: madsodgaard

SwiftKitCore/src/main/java/org/swift/swiftkitcore/ConfinedSwiftMemorySession.java

@@ -58,7 +58,7 @@ public void close() {
     public void register(SwiftInstance instance) {
         checkValid();
 
-        SwiftInstanceCleanup cleanup = new SwiftInstanceCleanup(instance);
+        SwiftInstanceCleanup cleanup = instance.makeCleanupAction();
         this.resources.add(cleanup);
     }
 

SwiftKitCore/src/main/java/org/swift/swiftkitcore/JNISwiftInstance.java

@@ -0,0 +1,28 @@
+package org.swift.swiftkitcore;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+public class JNISwiftInstance extends SwiftInstance {
+    /**
+     * The designated constructor of any imported Swift types.
+     *
+     * @param pointer a pointer to the memory containing the value
+     * @param arena   the arena this object belongs to. When the arena goes out of scope, this value is destroyed.
+     */
+    protected JNISwiftInstance(long pointer, SwiftArena arena) {
+        super(pointer, arena);
+    }
+
+    @Override
+    public SwiftInstanceCleanup makeCleanupAction() {
+        final AtomicBoolean statusDestroyedFlag = $statusDestroyedFlag();
+        Runnable markAsDestroyed = new Runnable() {
+            @Override
+            public void run() {
+                statusDestroyedFlag.set(true);
+            }
+        };
+
+        return new JNISwiftInstanceCleanup(this.getClass(), this.pointer(), markAsDestroyed);
+    }
+}

SwiftKitCore/src/main/java/org/swift/swiftkitcore/JNISwiftInstanceCleanup.java

@@ -0,0 +1,28 @@
+package org.swift.swiftkitcore;
+
+import java.lang.reflect.Method;
+
+class JNISwiftInstanceCleanup implements SwiftInstanceCleanup {
+    private final Class<? extends SwiftInstance> clazz;
+    private final long selfPointer;
+    private final Runnable markAsDestroyed;
+
+    public JNISwiftInstanceCleanup(Class<? extends SwiftInstance> clazz, long selfPointer, Runnable markAsDestroyed) {
+        this.clazz = clazz;
+        this.selfPointer = selfPointer;
+        this.markAsDestroyed = markAsDestroyed;
+    }
+
+    @Override
+    public void run() {
+        markAsDestroyed.run();
+
+        try {
+            // Use reflection to look for the static destroy method on the wrapper.
+            Method method = clazz.getDeclaredMethod("destroy", long.class);
+            method.invoke(null, selfPointer);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

SwiftKitCore/src/main/java/org/swift/swiftkitcore/SwiftInstance.java

@@ -29,8 +29,10 @@ public final long pointer() {
 
     /**
      * Called when the arena has decided the value should be destroyed.
+     * <p/>
+     * <b>Warning:</b> The cleanup action must not capture {@code this}.
      */
-    public abstract void destroy();
+    public abstract SwiftInstanceCleanup makeCleanupAction();
 
     // TODO: make this a flagset integer and/or use a field updater
     /** Used to track additional state of the underlying object, e.g. if it was explicitly destroyed. */

SwiftKitCore/src/main/java/org/swift/swiftkitcore/SwiftInstanceCleanup.java

@@ -17,19 +17,4 @@
 /**
  * A Swift memory instance cleanup, e.g. count-down a reference count and destroy a class, or destroy struct/enum etc.
  */
-public final class SwiftInstanceCleanup implements Runnable {
-    // TODO: Should this be a weak reference?
-    private final SwiftInstance swiftInstance;
-
-    public SwiftInstanceCleanup(SwiftInstance swiftInstance) {
-        this.swiftInstance = swiftInstance;
-    }
-
-    @Override
-    public void run() {
-        swiftInstance.$statusDestroyedFlag().set(true);
-
-        // System.out.println("[debug] Destroy swift value [" + selfType.getSwiftName() + "]: " + selfPointer);
-        swiftInstance.destroy();
-    }
-}
+public interface SwiftInstanceCleanup extends Runnable {}

SwiftKitFFM/src/main/java/org/swift/swiftkitffm/AllocatingSwiftArena.java

@@ -14,8 +14,8 @@ static ClosableAllocatingSwiftArena ofConfined() {
         return new FFMConfinedSwiftMemorySession(Thread.currentThread());
     }
 
-//    static SwiftArena ofAuto() {
-//        ThreadFactory cleanerThreadFactory = r -> new Thread(r, "AutoSwiftArenaCleanerThread");
-//        return new AutoSwiftMemorySession(cleanerThreadFactory);
-//    }
+    static AllocatingSwiftArena ofAuto() {
+        ThreadFactory cleanerThreadFactory = r -> new Thread(r, "AutoSwiftArenaCleanerThread");
+        return new AllocatingAutoSwiftMemorySession(cleanerThreadFactory);
+    }
 }

SwiftKitFFM/src/main/java/org/swift/swiftkitffm/AutoSwiftMemorySession.java

@@ -40,12 +40,12 @@
  *
  * <p> Whenever possible, prefer using an explicitly managed {@link SwiftArena}, such as {@link SwiftArena#ofConfined()}.
  */
-final class AutoSwiftMemorySession implements AllocatingSwiftArena {
+final class AllocatingAutoSwiftMemorySession implements AllocatingSwiftArena {
 
     private final Arena arena;
     private final Cleaner cleaner;
 
-    public AutoSwiftMemorySession(ThreadFactory cleanerThreadFactory) {
+    public AllocatingAutoSwiftMemorySession(ThreadFactory cleanerThreadFactory) {
         this.cleaner = Cleaner.create(cleanerThreadFactory);
         this.arena = Arena.ofAuto();
     }
@@ -54,8 +54,9 @@ public AutoSwiftMemorySession(ThreadFactory cleanerThreadFactory) {
     public void register(SwiftInstance instance) {
         Objects.requireNonNull(instance, "value");
 
-        // TODO: Captures warning???
-        var cleanupAction = new SwiftInstanceCleanup(instance);
+        // We make sure we don't capture `instance` in the
+        // cleanup action, so we can ignore the warning below.
+        var cleanupAction = instance.makeCleanupAction();
         cleaner.register(instance, cleanupAction);
     }
 

SwiftKitFFM/src/main/java/org/swift/swiftkitffm/FFMSwiftInstance.java

@@ -16,6 +16,7 @@
 
 import org.swift.swiftkitcore.SwiftArena;
 import org.swift.swiftkitcore.SwiftInstance;
+import org.swift.swiftkitcore.SwiftInstanceCleanup;
 
 import java.lang.foreign.GroupLayout;
 import java.lang.foreign.MemorySegment;
@@ -45,10 +46,14 @@ protected FFMSwiftInstance(MemorySegment segment, AllocatingSwiftArena arena) {
     }
 
     @Override
-    public void destroy() {
-        System.out.println("[debug] Destroy swift value [" + $swiftType().getSwiftName() + "]: " + $memorySegment());
-
-        // In FFM we allocate on the Java-side, so we can just call destroy.
-        SwiftValueWitnessTable.destroy(this.$swiftType(), this.$memorySegment());
+    public SwiftInstanceCleanup makeCleanupAction() {
+        var statusDestroyedFlag = $statusDestroyedFlag();
+        Runnable markAsDestroyed = () -> statusDestroyedFlag.set(true);
+
+        return new FFMSwiftInstanceCleanup(
+                $memorySegment(),
+                $swiftType(),
+                markAsDestroyed
+        );
     }
 }

SwiftKitFFM/src/main/java/org/swift/swiftkitffm/FFMSwiftInstanceCleanup.java

@@ -0,0 +1,28 @@
+package org.swift.swiftkitffm;
+
+import org.swift.swiftkitcore.SwiftInstanceCleanup;
+
+import java.lang.foreign.MemorySegment;
+
+public class FFMSwiftInstanceCleanup implements SwiftInstanceCleanup {
+    private final MemorySegment selfPointer;
+    private final SwiftAnyType selfType;
+    private final Runnable markAsDestroyed;
+
+    public FFMSwiftInstanceCleanup(MemorySegment selfPointer, SwiftAnyType selfType, Runnable markAsDestroyed) {
+        this.selfPointer = selfPointer;
+        this.selfType = selfType;
+        this.markAsDestroyed = markAsDestroyed;
+    }
+
+    @Override
+    public void run() {
+        markAsDestroyed.run();
+
+        // Allow null pointers just for AutoArena tests.
+        if (selfType != null && selfPointer != null) {
+            System.out.println("[debug] Destroy swift value [" + selfType.getSwiftName() + "]: " + selfPointer);
+            SwiftValueWitnessTable.destroy(selfType, selfPointer);
+        }
+    }
+}

SwiftKitFFM/src/main/java/org/swift/swiftkitffm/SwiftFFM.java

@@ -16,7 +16,6 @@
 
 import org.swift.swiftkitcore.SwiftHeapObject;
 import org.swift.swiftkitcore.util.PlatformUtils;
-import org.swift.swiftkitffm.util.PlatformUtils;
 
 import java.lang.foreign.*;
 import java.lang.invoke.MethodHandle;