Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ extension JNISwift2JavaGenerator {
"org.swift.swiftkit.core.util.*",
"org.swift.swiftkit.core.collections.*",
"java.util.*",
"java.util.concurrent.atomic.AtomicBoolean",

// NonNull, Unsigned and friends
"org.swift.swiftkit.core.annotations.*",
Expand Down Expand Up @@ -282,6 +281,7 @@ extension JNISwift2JavaGenerator {
}
printer.print(
"""
this.$cleanup = $createCleanup();

// Only register once we have fully initialized the object since this will need the object pointer.
swiftArena.register(this);
Expand Down Expand Up @@ -317,16 +317,16 @@ extension JNISwift2JavaGenerator {
/** Pointer to the "self". */
private final long selfPointer;

/** Used to track additional state of the underlying object, e.g. if it was explicitly destroyed. */
private final AtomicBoolean $state$destroyed = new AtomicBoolean(false);
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alloc we're removing; instead we have the Cleanup as we did before; alloc it early, but then inside the Cleanup we have no other heap objects -- just the int.

/** Tracks whether this instance has been destroyed; doubles as the destroyed-state holder. */
private final SwiftInstanceCleanup $cleanup;

public long $memoryAddress() {
return this.selfPointer;
}

@Override
public AtomicBoolean $statusDestroyedFlag() {
return $state$destroyed;
public SwiftInstanceCleanup $cleanup() {
return $cleanup;
}
"""
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void register(SwiftInstance instance) {

// We make sure we don't capture `instance` in the
// cleanup action, so we can ignore the warning below.
var cleanupAction = instance.$createCleanup();
var cleanupAction = instance.$cleanup();
swiftCleaner.register(instance, cleanupAction);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void close() {
public void register(SwiftInstance instance) {
checkValid();

SwiftInstanceCleanup cleanup = instance.$createCleanup();
SwiftInstanceCleanup cleanup = instance.$cleanup();
this.resources.add(cleanup);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ public interface JNISwiftInstance extends SwiftInstance {
*/
long $typeMetadataAddress();

@Override
/**
* Build a fresh {@link SwiftInstanceCleanup} for this instance.
* <p>
* Concrete implementations are expected to call this once during construction
* and cache the result in a field exposed via {@link #$cleanup()}.
*/
default SwiftInstanceCleanup $createCleanup() {
return new JNISwiftInstanceCleanup(
$createDestroyFunction(),
$statusDestroyedFlag()
);
return new JNISwiftInstanceCleanup($createDestroyFunction());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,29 @@

package org.swift.swiftkit.core;

import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;

class JNISwiftInstanceCleanup implements SwiftInstanceCleanup {
private static final AtomicIntegerFieldUpdater<JNISwiftInstanceCleanup> DESTROYED =
AtomicIntegerFieldUpdater.newUpdater(JNISwiftInstanceCleanup.class, "destroyed");

private final Runnable destroyFunction;
private final AtomicBoolean statusDestroyedFlag;

public JNISwiftInstanceCleanup(Runnable destroyFunction, AtomicBoolean statusDestroyedFlag) {
@SuppressWarnings("unused") // accessed via DESTROYED field updater
private volatile int destroyed;

public JNISwiftInstanceCleanup(Runnable destroyFunction) {
this.destroyFunction = destroyFunction;
this.statusDestroyedFlag = statusDestroyedFlag;
}

@Override
public boolean isDestroyed() {
return destroyed != 0;
}

@Override
public void run() {
if (statusDestroyedFlag.compareAndSet(false, true)) {
if (DESTROYED.compareAndSet(this, 0, 1)) {
destroyFunction.run();
} else {
throw new IllegalStateException("Double destruction attempt detected!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

package org.swift.swiftkit.core;

import java.util.concurrent.atomic.AtomicBoolean;

public interface SwiftInstance {
/**
* Pointer to the {@code self} of the underlying Swift object or value.
Expand All @@ -27,20 +25,16 @@ public interface SwiftInstance {
long $memoryAddress();

/**
* Called when the arena has decided the value should be destroyed.
* Returns the cleanup associated with this instance.
* <p>
* <b>Warning:</b> The cleanup action must not capture {@code this}.
*/
SwiftInstanceCleanup $createCleanup();

/**
* Exposes a boolean value which can be used to indicate if the object was destroyed.
* The same cleanup instance is returned on every call. The cleanup also serves as the
* destroyed-state holder, allowing callers to poll {@link SwiftInstanceCleanup#isDestroyed()}
* even after this instance has been GC-ed.
* <p>
* This is exposing the object, rather than performing the action because we don't want to accidentally
* form a strong reference to the {@code SwiftInstance} which could prevent the cleanup from running,
* if using an GC managed instance (e.g. using an {@code AutoSwiftMemorySession}.
* <b>Warning:</b> The cleanup must not capture {@code this}.
*/
AtomicBoolean $statusDestroyedFlag();
SwiftInstanceCleanup $cleanup();

/**
* Ensures that this instance has not been destroyed.
* <p>
Expand All @@ -49,7 +43,7 @@ public interface SwiftInstance {
* use-after-free errors.
*/
default void $ensureAlive() {
if (this.$statusDestroyedFlag().get()) {
if (this.$cleanup().isDestroyed()) {
throw new IllegalStateException("Attempted to call method on already destroyed instance of " + getClass().getSimpleName() + "!");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,13 @@

/**
* A Swift memory instance cleanup, e.g. count-down a reference count and destroy a class, or destroy struct/enum etc.
* <p>
* Implementations also serve as the destroyed-state holder for their associated {@link SwiftInstance},
* exposing {@link #isDestroyed()} which can be polled even after the instance has been GC-ed.
*/
public interface SwiftInstanceCleanup extends Runnable {}
public interface SwiftInstanceCleanup extends Runnable {
/**
* Whether this cleanup has run, i.e. the associated instance has been destroyed.
*/
boolean isDestroyed();
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package org.swift.swiftkit.core.collections;

import java.util.*;
import java.util.concurrent.atomic.AtomicBoolean;

import org.swift.swiftkit.core.*;

Expand All @@ -33,10 +32,11 @@
public class SwiftDictionaryMap<K, V> extends AbstractMap<K, V> implements JNISwiftInstance {

private final long selfPointer;
private final AtomicBoolean destroyed = new AtomicBoolean(false);
private final SwiftInstanceCleanup cleanup;

private SwiftDictionaryMap(long selfPointer) {
this.selfPointer = selfPointer;
this.cleanup = $createCleanup();
}

public static <K, V> SwiftDictionaryMap<K, V> wrapMemoryAddressUnsafe(long selfPointer, SwiftArena arena) {
Expand All @@ -58,8 +58,8 @@ public static <K, V> SwiftDictionaryMap<K, V> wrapMemoryAddressUnsafe(long selfP
}

@Override
public AtomicBoolean $statusDestroyedFlag() {
return destroyed;
public SwiftInstanceCleanup $cleanup() {
return cleanup;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package org.swift.swiftkit.core.collections;

import java.util.*;
import java.util.concurrent.atomic.AtomicBoolean;

import org.swift.swiftkit.core.*;

Expand All @@ -32,10 +31,11 @@
public class SwiftSet<E> extends AbstractSet<E> implements JNISwiftInstance {

private final long selfPointer;
private final AtomicBoolean destroyed = new AtomicBoolean(false);
private final SwiftInstanceCleanup cleanup;

private SwiftSet(long selfPointer) {
this.selfPointer = selfPointer;
this.cleanup = $createCleanup();
}

public static <E> SwiftSet<E> wrapMemoryAddressUnsafe(long selfPointer, SwiftArena arena) {
Expand All @@ -57,8 +57,8 @@ public static <E> SwiftSet<E> wrapMemoryAddressUnsafe(long selfPointer, SwiftAre
}

@Override
public AtomicBoolean $statusDestroyedFlag() {
return destroyed;
public SwiftInstanceCleanup $cleanup() {
return cleanup;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
import org.junit.jupiter.api.Test;
import org.swift.swiftkit.core.JNISwiftInstance;
import org.swift.swiftkit.core.SwiftArena;

import java.util.concurrent.atomic.AtomicBoolean;
import org.swift.swiftkit.core.SwiftInstanceCleanup;

public class AutoArenaTest {

Expand All @@ -29,15 +28,15 @@ public void cleaner_releases_native_resource() {

// This object is registered to the arena.
var object = new FakeSwiftInstance(arena);
var statusDestroyedFlag = object.$statusDestroyedFlag();
var cleanup = object.$cleanup();

// Release the object and hope it gets GC-ed soon

// noinspection UnusedAssignment
object = null;

var i = 1_000;
while (!statusDestroyedFlag.get()) {
while (!cleanup.isDestroyed()) {
System.runFinalization();
System.gc();

Expand All @@ -48,9 +47,10 @@ public void cleaner_releases_native_resource() {
}

private static class FakeSwiftInstance implements JNISwiftInstance {
AtomicBoolean $state$destroyed = new AtomicBoolean(false);
private final SwiftInstanceCleanup cleanup;

public FakeSwiftInstance(SwiftArena arena) {
this.cleanup = $createCleanup();
arena.register(this);
}

Expand All @@ -69,8 +69,8 @@ public FakeSwiftInstance(SwiftArena arena) {
}

@Override
public AtomicBoolean $statusDestroyedFlag() {
return $state$destroyed;
public SwiftInstanceCleanup $cleanup() {
return cleanup;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void register(SwiftInstance instance) {

// We make sure we don't capture `instance` in the
// cleanup action, so we can ignore the warning below.
var cleanupAction = instance.$createCleanup();
var cleanupAction = instance.$cleanup();
cleaner.register(instance, cleanupAction);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.swift.swiftkit.core.SwiftInstanceCleanup;

import java.lang.foreign.MemorySegment;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* Base class for Swift errors passed across the FFM boundary.
Expand All @@ -27,17 +26,19 @@
*/
public abstract class FFMSwiftErrorInstance extends Exception implements SwiftInstance {
private final MemorySegment memorySegment;
private final AtomicBoolean $state$destroyed = new AtomicBoolean(false);
private final FFMSwiftInstanceCleanup cleanup;

protected FFMSwiftErrorInstance(String message, MemorySegment segment, AllocatingSwiftArena arena) {
super(message);
this.memorySegment = segment;
this.cleanup = new FFMSwiftInstanceCleanup(segment, $swiftType());
arena.register(this);
}

protected FFMSwiftErrorInstance(MemorySegment segment, AllocatingSwiftArena arena) {
super();
this.memorySegment = segment;
this.cleanup = new FFMSwiftInstanceCleanup(segment, $swiftType());
arena.register(this);
}

Expand All @@ -50,21 +51,13 @@ protected FFMSwiftErrorInstance(MemorySegment segment, AllocatingSwiftArena aren
return $memorySegment().address();
}

public final AtomicBoolean $statusDestroyedFlag() {
return $state$destroyed;
@Override
public final SwiftInstanceCleanup $cleanup() {
return this.cleanup;
}

/**
* The Swift type metadata of this type.
*/
public abstract SwiftAnyType $swiftType();

@Override
public SwiftInstanceCleanup $createCleanup() {
return new FFMSwiftInstanceCleanup(
$memorySegment(),
$swiftType(),
$statusDestroyedFlag()
);
}
}
Loading
Loading