From 3af7a20d20ece5899f7165d9dcca781026f57a6d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:04:33 +0000 Subject: [PATCH 1/3] Initial plan From 3f736b5222009549679f60754519950dd7faa514 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 16:25:54 +0000 Subject: [PATCH 2/3] feat(java): add JNA binding interface and implementation (task 4.4) - Add JNA 5.19.1 as optional dependency to java/sdk/pom.xml with jna.version property for deliberate upgrades - Create OutboundCallback.java: JNA Callback interface for native-to-Java outbound data delivery - Create NativeBinding.java: interface abstraction for the 5 copilot_runtime_* C ABI entry points - Create JnaNativeBinding.java: JNA implementation with static singleton (library-never-unloads pattern), duplicate path guard, and active-callback AtomicInteger tracking - Create JnaNativeBindingTest.java: 24 unit tests covering delegation, loading, duplicate guard, and callback behavior using the spike-3-4 test native library - Update module-info.java: requires static com.sun.jna, opens com.github.copilot.ffi to com.sun.jna Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- java/sdk/pom.xml | 14 + .../github/copilot/ffi/JnaNativeBinding.java | 212 +++++++ .../com/github/copilot/ffi/NativeBinding.java | 131 +++++ .../github/copilot/ffi/OutboundCallback.java | 46 ++ java/sdk/src/main/java/module-info.java | 2 + .../copilot/ffi/JnaNativeBindingTest.java | 545 ++++++++++++++++++ 6 files changed, 950 insertions(+) create mode 100644 java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java create mode 100644 java/sdk/src/main/java/com/github/copilot/ffi/NativeBinding.java create mode 100644 java/sdk/src/main/java/com/github/copilot/ffi/OutboundCallback.java create mode 100644 java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java diff --git a/java/sdk/pom.xml b/java/sdk/pom.xml index e24c2ebf2e..53b15e3f68 100644 --- a/java/sdk/pom.xml +++ b/java/sdk/pom.xml @@ -62,6 +62,8 @@ false + + 5.19.1 @@ -90,6 +92,18 @@ provided + + + net.java.dev.jna + jna + ${jna.version} + true + + org.junit.jupiter diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java b/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java new file mode 100644 index 0000000000..70536910a6 --- /dev/null +++ b/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java @@ -0,0 +1,212 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import com.sun.jna.Library; +import com.sun.jna.Native; +import com.sun.jna.Pointer; + +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; + +/** + * JNA-backed implementation of {@link NativeBinding}. + * + *

+ * Loads the {@code runtime.node} native library by absolute path and delegates + * each {@link NativeBinding} method to the corresponding + * {@code copilot_runtime_*} C ABI export. + * + *

Library-never-unloads pattern

+ *

+ * The loaded JNA library handle is held in a {@code static} field and is never + * released. Native worker threads spawned by the runtime outlive any individual + * {@code FfiRuntimeHost} instance; unloading the library while those threads + * are active would cause a crash. This mirrors the Rust runtime's own + * {@code OnceLock>>} pattern. + * + *

Duplicate-load guard

+ *

+ * Loading a library from a different absolute path in the same JVM + * process is rejected with {@link IllegalStateException}. Loading from the + * same path more than once is silently accepted. + * + *

Active-callback tracking

+ *

+ * The {@link #activeCallbacks} counter is incremented when the native runtime + * enters the outbound callback and decremented when the callback returns. + * Callers (e.g. {@code FfiRuntimeHost}) must drain this counter to zero before + * calling {@link #connectionClose} or {@link #hostShutdown}. + * + *

GraalVM Native Image

+ *

+ * JNA callback upcalls are not supported under GraalVM Native Image. InProcess + * transport is not available in native-image executables; use subprocess + * transport instead. + */ +final class JnaNativeBinding implements NativeBinding { + + private static final Logger LOG = Logger.getLogger(JnaNativeBinding.class.getName()); + + /** + * JNA inner interface mapping the five {@code copilot_runtime_*} C ABI exports. + */ + interface CopilotRuntimeLibrary extends Library { + /** Corresponds to {@code copilot_runtime_host_start}. */ + int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen); + + /** Corresponds to {@code copilot_runtime_host_shutdown}. */ + boolean copilot_runtime_host_shutdown(int serverId); + + /** Corresponds to {@code copilot_runtime_connection_open}. */ + int copilot_runtime_connection_open(int serverId, OutboundCallback callback, Pointer userData, byte[] extSource, + int extSourceLen, byte[] extName, int extNameLen, byte[] connToken, int connTokenLen); + + /** Corresponds to {@code copilot_runtime_connection_write}. */ + boolean copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen); + + /** Corresponds to {@code copilot_runtime_connection_close}. */ + boolean copilot_runtime_connection_close(int connectionId); + } + + // ------------------------------------------------------------------------- + // Process-wide singleton — never unloaded + // ------------------------------------------------------------------------- + + private static final Object LOAD_LOCK = new Object(); + + /** Absolute path of the library that was first loaded into this JVM process. */ + private static volatile Path loadedPath; + + /** The loaded JNA library interface. Never released after first set. */ + private static volatile CopilotRuntimeLibrary loadedLib; + + // ------------------------------------------------------------------------- + // Instance state + // ------------------------------------------------------------------------- + + /** + * The library interface used by this instance for all delegated calls. + * + *

+ * For the production path ({@link #JnaNativeBinding(Path)}), this is always the + * same object as {@link #loadedLib} (the static singleton). For the test path + * ({@link #JnaNativeBinding(CopilotRuntimeLibrary)}), this may be a stub or + * mock without modifying the static singleton. + */ + private final CopilotRuntimeLibrary lib; + + /** + * Count of callbacks currently executing on native threads. Must reach zero + * before {@link #connectionClose} or {@link #hostShutdown} is called. + */ + final AtomicInteger activeCallbacks = new AtomicInteger(0); + + // ------------------------------------------------------------------------- + // Constructors + // ------------------------------------------------------------------------- + + /** + * Loads (or re-uses) the native library at the given absolute path. + * + * @param libraryPath + * absolute path to the {@code runtime.node} native library + * @throws IllegalStateException + * if a different library path has already been loaded in + * this JVM process + */ + JnaNativeBinding(Path libraryPath) { + Path absPath = libraryPath.toAbsolutePath().normalize(); + synchronized (LOAD_LOCK) { + if (loadedLib == null) { + LOG.fine(() -> "Loading native library from: " + absPath); + loadedLib = Native.load(absPath.toString(), CopilotRuntimeLibrary.class); + loadedPath = absPath; + LOG.fine(() -> "Native library loaded: " + absPath); + } else if (!absPath.equals(loadedPath)) { + throw new IllegalStateException("An in-process FFI runtime library is already loaded from '" + + loadedPath + "'; loading a different library from '" + absPath + + "' in the same process is not supported."); + } + } + this.lib = loadedLib; + } + + /** + * Testing constructor — accepts a pre-built {@link CopilotRuntimeLibrary} + * directly, bypassing disk I/O and the static singleton guard. + * + *

+ * This constructor is package-private and intended solely for unit tests. + * + * @param library + * a {@link CopilotRuntimeLibrary} stub or mock for testing + */ + JnaNativeBinding(CopilotRuntimeLibrary library) { + // Testing seam — skip the static singleton guard. + this.lib = library; + } + + // ------------------------------------------------------------------------- + // NativeBinding delegation + // ------------------------------------------------------------------------- + + @Override + public int hostStart(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen) { + return lib.copilot_runtime_host_start(argvJson, argvJsonLen, envJson, envJsonLen); + } + + @Override + public boolean hostShutdown(int serverId) { + return lib.copilot_runtime_host_shutdown(serverId); + } + + @Override + public int connectionOpen(int serverId, OutboundCallback callback, Pointer userData, byte[] extSource, + int extSourceLen, byte[] extName, int extNameLen, byte[] connToken, int connTokenLen) { + // Wrap the caller's callback to maintain active-callback tracking. + OutboundCallback tracked = (ud, data, len) -> { + activeCallbacks.incrementAndGet(); + try { + callback.invoke(ud, data, len); + } finally { + activeCallbacks.decrementAndGet(); + } + }; + return lib.copilot_runtime_connection_open(serverId, tracked, userData, extSource, extSourceLen, extName, + extNameLen, connToken, connTokenLen); + } + + @Override + public boolean connectionWrite(int connectionId, byte[] data, int dataLen) { + return lib.copilot_runtime_connection_write(connectionId, data, dataLen); + } + + @Override + public boolean connectionClose(int connectionId) { + return lib.copilot_runtime_connection_close(connectionId); + } + + // ------------------------------------------------------------------------- + // Testing support + // ------------------------------------------------------------------------- + + /** + * Resets the process-wide static state for unit tests. + * + *

+ * Must only be called from test code. Resets + * {@link #loadedPath} and {@link #loadedLib} so that a subsequent + * {@link #JnaNativeBinding(Path)} call can load a different library. In + * production, the library is never unloaded. + */ + static void resetForTesting() { + synchronized (LOAD_LOCK) { + loadedPath = null; + loadedLib = null; + } + } +} diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeBinding.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeBinding.java new file mode 100644 index 0000000000..3aa8ca9e4d --- /dev/null +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeBinding.java @@ -0,0 +1,131 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import com.sun.jna.Pointer; + +/** + * Internal abstraction over the Copilot runtime C ABI. + * + *

+ * Defines the five {@code extern "C"} entry points exposed by the native + * {@code runtime.node} library. The JNA-backed implementation + * ({@link JnaNativeBinding}) delegates to these through JNA. A future FFM + * implementation may be substituted via the multi-release JAR mechanism without + * changing callers. + * + *

+ * All classes in {@code com.github.copilot.ffi} are internal; consumers must + * not reference them directly. + * + *

C ABI entry points

+ *
    + *
  • {@code copilot_runtime_host_start} — start the runtime host
  • + *
  • {@code copilot_runtime_host_shutdown} — shut down the runtime host
  • + *
  • {@code copilot_runtime_connection_open} — open a bidirectional + * connection
  • + *
  • {@code copilot_runtime_connection_write} — write a JSON-RPC frame to the + * runtime
  • + *
  • {@code copilot_runtime_connection_close} — close a connection
  • + *
+ * + *

Wire format

+ *

+ * All frames use LSP {@code Content-Length} header framing, identical to the + * stdio transport. No special encoding or decoding is needed at the FFI + * boundary. + */ +interface NativeBinding { + + /** + * Starts the runtime host. + * + *

+ * Blocks for up to ~30 s while the worker boots and connects back. Must not be + * called on an async/reactive executor thread. + * + * @param argvJson + * UTF-8 JSON array of strings: the entrypoint and required flags + * @param argvJsonLen + * byte length of {@code argvJson} + * @param envJson + * UTF-8 JSON object of environment overrides, or {@code null} when + * empty + * @param envJsonLen + * byte length of {@code envJson}, or {@code 0} when {@code envJson} + * is null + * @return server handle ({@code 0} on failure) + */ + int hostStart(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen); + + /** + * Shuts down the runtime host. + * + * @param serverId + * non-zero server handle returned by {@link #hostStart} + * @return {@code true} on success + */ + boolean hostShutdown(int serverId); + + /** + * Opens a bidirectional connection and registers the outbound data callback. + * + *

+ * The {@code extSource}, {@code extName}, and {@code connToken} parameters are + * reserved extension points. All current SDK implementations pass + * {@code null}/0 for all three. + * + * @param serverId + * non-zero server handle returned by {@link #hostStart} + * @param callback + * JNA callback invoked by the runtime on native threads when + * outbound data is available; must be held as a strong reference by + * the caller + * @param userData + * opaque cookie passed back to {@code callback} unchanged; pass + * {@link Pointer#NULL} + * @param extSource + * reserved; pass {@code null} + * @param extSourceLen + * byte length of {@code extSource}; pass {@code 0} + * @param extName + * reserved; pass {@code null} + * @param extNameLen + * byte length of {@code extName}; pass {@code 0} + * @param connToken + * reserved; pass {@code null} + * @param connTokenLen + * byte length of {@code connToken}; pass {@code 0} + * @return connection handle ({@code 0} on failure) + */ + int connectionOpen(int serverId, OutboundCallback callback, Pointer userData, byte[] extSource, int extSourceLen, + byte[] extName, int extNameLen, byte[] connToken, int connTokenLen); + + /** + * Writes a JSON-RPC frame to the runtime. + * + *

+ * The native side copies the buffer synchronously before returning; the byte + * array does not need to survive past this call. + * + * @param connectionId + * non-zero connection handle returned by {@link #connectionOpen} + * @param data + * frame bytes + * @param dataLen + * byte length of {@code data} + * @return {@code true} on success + */ + boolean connectionWrite(int connectionId, byte[] data, int dataLen); + + /** + * Closes a connection. + * + * @param connectionId + * non-zero connection handle returned by {@link #connectionOpen} + * @return {@code true} on success + */ + boolean connectionClose(int connectionId); +} diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/OutboundCallback.java b/java/sdk/src/main/java/com/github/copilot/ffi/OutboundCallback.java new file mode 100644 index 0000000000..6f5c319224 --- /dev/null +++ b/java/sdk/src/main/java/com/github/copilot/ffi/OutboundCallback.java @@ -0,0 +1,46 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import com.sun.jna.Callback; +import com.sun.jna.Pointer; + +/** + * JNA callback interface for the runtime-to-Java outbound data path. + * + *

+ * The native runtime invokes this callback on a native thread when data is + * ready to be delivered to the Java side. JNA automatically attaches the native + * thread to the JVM before dispatching the callback. + * + *

+ * Buffer lifetime: The {@code data} pointer is only valid for + * the duration of the callback invocation. Implementations must copy the bytes + * out (e.g. {@code data.getByteArray(0, len)}) before returning. + * + *

+ * GC protection: Instances must be held as strong-reference + * fields for as long as native code may invoke the callback. If the instance is + * garbage-collected, the function pointer becomes dangling and the JVM will + * crash. + */ +@FunctionalInterface +interface OutboundCallback extends Callback { + + /** + * Invoked by the native runtime when outbound data is available. + * + * @param userData + * opaque cookie passed through unchanged from + * {@code copilot_runtime_connection_open}; always + * {@code Pointer.NULL} in this SDK + * @param data + * pointer to the outbound byte buffer; valid only for the duration + * of this invocation + * @param len + * byte length of the buffer pointed to by {@code data} + */ + void invoke(Pointer userData, Pointer data, int len); +} diff --git a/java/sdk/src/main/java/module-info.java b/java/sdk/src/main/java/module-info.java index 38bc1f93d5..8bc2dbd55c 100644 --- a/java/sdk/src/main/java/module-info.java +++ b/java/sdk/src/main/java/module-info.java @@ -12,6 +12,7 @@ requires com.fasterxml.jackson.datatype.jsr310; requires static com.github.spotbugs.annotations; requires static java.compiler; + requires static com.sun.jna; requires java.net.http; requires java.logging; @@ -25,6 +26,7 @@ opens com.github.copilot.generated to com.fasterxml.jackson.databind; opens com.github.copilot.generated.rpc to com.fasterxml.jackson.databind; opens com.github.copilot.rpc to com.fasterxml.jackson.databind; + opens com.github.copilot.ffi to com.sun.jna; provides javax.annotation.processing.Processor with com.github.copilot.CopilotExperimentalProcessor, com.github.copilot.tool.CopilotToolProcessor; diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java new file mode 100644 index 0000000000..790c2607f7 --- /dev/null +++ b/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java @@ -0,0 +1,545 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.ffi; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.sun.jna.Library; +import com.sun.jna.Native; +import com.sun.jna.Pointer; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Unit tests for {@link JnaNativeBinding} using the spike-3-4 test native + * library ({@code libcallback_test}). + * + *

+ * The spike test library exports simplified versions of the runtime ABI + * functions: {@code host_start}, {@code host_shutdown}, + * {@code connection_open}, {@code connection_write}, and + * {@code connection_close}. Callback tests use this library through a + * test-specific JNA interface, while loading and guard tests exercise + * {@link JnaNativeBinding} directly. + * + *

+ * Tests that require the native library are conditionally skipped when the + * library is not present (e.g. on an architecture without a pre-built binary). + */ +class JnaNativeBindingTest { + + /** + * System property that points at the absolute path of the test native library. + * + *

+ * Default: the {@code libcallback_test.so} built from the spike-3-4 Rust crate, + * relative to {@code java/sdk/}. + */ + private static final String TEST_LIB_PATH_PROP = "copilot.test.nativelib.path"; + + private static final String SPIKE_LIB_PATH = System.getProperty(TEST_LIB_PATH_PROP, + "../../1917-java-embed-rust-cli-runtime-remove-before-merge" + "/spike-3-4-jna-callback-and-threading" + + "/rust-dll/target/release/libcallback_test.so"); + + // ------------------------------------------------------------------------- + // Test-specific JNA interface for the spike-3-4 test library + // ------------------------------------------------------------------------- + + /** + * JNA interface for the simplified test library. Maps Java names to the + * snake_case exports of {@code libcallback_test}. + */ + interface CallbackTestLib extends Library { + /** Simulates {@code copilot_runtime_host_start}; always returns 42. */ + int host_start(); + + /** + * Simulates {@code copilot_runtime_host_shutdown}; always returns {@code true}. + */ + boolean host_shutdown(int serverHandle); + + /** + * Simulates {@code copilot_runtime_connection_open}. Spawns a native thread + * that invokes {@code callback} {@code burstCount} times. Returns 7. + */ + int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount); + + /** + * Simulates {@code copilot_runtime_connection_write}; always returns + * {@code true}. + */ + boolean connection_write(int connectionHandle, byte[] data, int len); + + /** + * Simulates {@code copilot_runtime_connection_close}; always returns + * {@code true}. + */ + boolean connection_close(int connectionHandle); + } + + // ------------------------------------------------------------------------- + // Stub CopilotRuntimeLibrary for delegation tests + // ------------------------------------------------------------------------- + + /** + * Minimal stub for testing {@link JnaNativeBinding} delegation without disk + * I/O. + */ + private static class StubRuntimeLibrary implements JnaNativeBinding.CopilotRuntimeLibrary { + int hostStartReturn = 1; + boolean hostShutdownReturn = true; + int connectionOpenReturn = 1; + boolean connectionWriteReturn = true; + boolean connectionCloseReturn = true; + + byte[] lastArgvJson; + int lastArgvJsonLen; + int lastServerId; + int lastConnectionId; + + @Override + public int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen) { + lastArgvJson = argvJson; + lastArgvJsonLen = argvJsonLen; + return hostStartReturn; + } + + @Override + public boolean copilot_runtime_host_shutdown(int serverId) { + lastServerId = serverId; + return hostShutdownReturn; + } + + @Override + public int copilot_runtime_connection_open(int serverId, OutboundCallback callback, Pointer userData, + byte[] extSource, int extSourceLen, byte[] extName, int extNameLen, byte[] connToken, + int connTokenLen) { + lastServerId = serverId; + return connectionOpenReturn; + } + + @Override + public boolean copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen) { + lastConnectionId = connectionId; + return connectionWriteReturn; + } + + @Override + public boolean copilot_runtime_connection_close(int connectionId) { + lastConnectionId = connectionId; + return connectionCloseReturn; + } + } + + // ------------------------------------------------------------------------- + // Helpers + // ------------------------------------------------------------------------- + + private static boolean testLibExists() { + return Files.isRegularFile(testLibAbsPath()); + } + + private static Path testLibAbsPath() { + return Path.of(SPIKE_LIB_PATH).toAbsolutePath().normalize(); + } + + private static CallbackTestLib loadTestLib() { + return Native.load(testLibAbsPath().toString(), CallbackTestLib.class); + } + + @AfterEach + void resetStaticState() { + JnaNativeBinding.resetForTesting(); + } + + // ========================================================================= + // Delegation via testing constructor (stub — no disk I/O) + // ========================================================================= + + @Test + void hostStartDelegatesToLibraryAndReturnsHandle() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.hostStartReturn = 77; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + byte[] argv = "[\"copilot\"]".getBytes(StandardCharsets.UTF_8); + int result = binding.hostStart(argv, argv.length, null, 0); + + assertEquals(77, result, "hostStart should return the stub's configured value"); + assertEquals(argv, stub.lastArgvJson, "argv bytes should be passed through unchanged"); + assertEquals(argv.length, stub.lastArgvJsonLen); + } + + @Test + void hostStartReturnsZeroOnFailure() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.hostStartReturn = 0; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + byte[] argv = "[\"copilot\"]".getBytes(StandardCharsets.UTF_8); + assertEquals(0, binding.hostStart(argv, argv.length, null, 0), "hostStart must return 0 to signal failure"); + } + + @Test + void hostShutdownDelegatesToLibrary() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.hostShutdownReturn = true; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + assertTrue(binding.hostShutdown(42)); + assertEquals(42, stub.lastServerId); + } + + @Test + void hostShutdownReturnsFalseOnFailure() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.hostShutdownReturn = false; + JnaNativeBinding binding = new JnaNativeBinding(stub); + assertFalse(binding.hostShutdown(1)); + } + + @Test + void connectionOpenDelegatesToLibrary() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionOpenReturn = 55; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + OutboundCallback noop = (ud, data, len) -> { + }; + int connId = binding.connectionOpen(42, noop, Pointer.NULL, null, 0, null, 0, null, 0); + + assertEquals(55, connId, "connectionOpen should return the stub's configured handle"); + assertEquals(42, stub.lastServerId); + } + + @Test + void connectionOpenReturnsZeroOnFailure() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionOpenReturn = 0; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + OutboundCallback noop = (ud, data, len) -> { + }; + assertEquals(0, binding.connectionOpen(1, noop, Pointer.NULL, null, 0, null, 0, null, 0), + "connectionOpen must return 0 to signal failure"); + } + + @Test + void connectionWriteDelegatesToLibrary() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionWriteReturn = true; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + byte[] data = "hello".getBytes(StandardCharsets.UTF_8); + assertTrue(binding.connectionWrite(7, data, data.length)); + assertEquals(7, stub.lastConnectionId); + } + + @Test + void connectionWriteReturnsFalseOnFailure() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionWriteReturn = false; + JnaNativeBinding binding = new JnaNativeBinding(stub); + + byte[] data = "x".getBytes(StandardCharsets.UTF_8); + assertFalse(binding.connectionWrite(1, data, data.length), + "connectionWrite must propagate false return from the library"); + } + + @Test + void connectionCloseDelegatesToLibrary() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionCloseReturn = true; + JnaNativeBinding binding = new JnaNativeBinding(stub); + assertTrue(binding.connectionClose(7)); + assertEquals(7, stub.lastConnectionId); + } + + @Test + void connectionCloseReturnsFalseOnFailure() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionCloseReturn = false; + JnaNativeBinding binding = new JnaNativeBinding(stub); + assertFalse(binding.connectionClose(1)); + } + + @Test + void activeCallbacksStartsAtZero() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + JnaNativeBinding binding = new JnaNativeBinding(stub); + assertEquals(0, binding.activeCallbacks.get(), "Active callback counter must start at zero"); + } + + // ========================================================================= + // Library loading — success paths (requires native library on disk) + // ========================================================================= + + @Test + void loadByPathSucceedsWhenLibraryExists() { + if (!testLibExists()) { + return; + } + JnaNativeBinding binding = new JnaNativeBinding(testLibAbsPath()); + assertNotNull(binding); + } + + @Test + void loadByPathTwiceWithSamePathSucceeds() { + if (!testLibExists()) { + return; + } + new JnaNativeBinding(testLibAbsPath()); + // Second construction with the same absolute path must not throw. + new JnaNativeBinding(testLibAbsPath()); + } + + @Test + void activeCallbacksStartsAtZeroAfterPathLoad() { + if (!testLibExists()) { + return; + } + JnaNativeBinding binding = new JnaNativeBinding(testLibAbsPath()); + assertEquals(0, binding.activeCallbacks.get()); + } + + // ========================================================================= + // Duplicate-load guard + // ========================================================================= + + @Test + void loadFromDifferentPathThrowsIllegalState(@TempDir Path tempDir) throws Exception { + if (!testLibExists()) { + return; + } + Path altPath = tempDir.resolve("libcallback_test_alt.so"); + Files.copy(testLibAbsPath(), altPath); + + new JnaNativeBinding(testLibAbsPath()); + + IllegalStateException ex = assertThrows(IllegalStateException.class, () -> new JnaNativeBinding(altPath)); + + String msg = ex.getMessage(); + assertTrue(msg.contains("already loaded from"), "Diagnostic must mention 'already loaded from', got: " + msg); + assertTrue(msg.contains(testLibAbsPath().toString()), "Diagnostic must contain path A, got: " + msg); + assertTrue(msg.contains(altPath.toString()), "Diagnostic must contain path B, got: " + msg); + } + + @Test + void duplicateLoadDiagnosticMentionsNotSupported(@TempDir Path tempDir) throws Exception { + if (!testLibExists()) { + return; + } + Path altPath = tempDir.resolve("libcallback_test_b.so"); + Files.copy(testLibAbsPath(), altPath); + + new JnaNativeBinding(testLibAbsPath()); + + IllegalStateException ex = assertThrows(IllegalStateException.class, () -> new JnaNativeBinding(altPath)); + assertTrue(ex.getMessage().contains("not supported"), + "Diagnostic must mention 'not supported', got: " + ex.getMessage()); + } + + @Test + void resetForTestingAllowsReloadFromDifferentPath(@TempDir Path tempDir) throws Exception { + if (!testLibExists()) { + return; + } + Path altPath = tempDir.resolve("libcallback_test_reset.so"); + Files.copy(testLibAbsPath(), altPath); + + new JnaNativeBinding(testLibAbsPath()); + + JnaNativeBinding.resetForTesting(); + + // After reset, a different path must succeed. + new JnaNativeBinding(altPath); + } + + // ========================================================================= + // Callback invocation via test native library + // ========================================================================= + + @Test + void callbackIsInvokedFromNativeThread() throws Exception { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + assertEquals(42, serverHandle, "host_start should return 42"); + + int burstCount = 3; + CountDownLatch latch = new CountDownLatch(burstCount); + AtomicInteger callbackCount = new AtomicInteger(0); + AtomicInteger activeCallbacks = new AtomicInteger(0); + + OutboundCallback callback = (userData, data, len) -> { + activeCallbacks.incrementAndGet(); + try { + callbackCount.incrementAndGet(); + // Copy before returning — pointer only valid during invocation. + byte[] bytes = data.getByteArray(0, len); + assertEquals(len, bytes.length, "Copied byte array length must equal len parameter"); + } finally { + activeCallbacks.decrementAndGet(); + latch.countDown(); + } + }; + + int connHandle = lib.connection_open(serverHandle, callback, Pointer.NULL, burstCount); + assertEquals(7, connHandle, "connection_open should return 7"); + + assertTrue(latch.await(10, TimeUnit.SECONDS), "All callbacks must complete within 10 seconds"); + assertEquals(burstCount, callbackCount.get(), "Callback must be invoked exactly burstCount times"); + assertEquals(0, activeCallbacks.get(), + "Active callback count must return to zero after all callbacks complete"); + } + + @Test + void activeCallbackCountIsIncrementedDuringCallback() throws Exception { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + + int burstCount = 1; + CountDownLatch enteredLatch = new CountDownLatch(burstCount); + CountDownLatch exitLatch = new CountDownLatch(burstCount); + AtomicInteger observedOnEntry = new AtomicInteger(-1); + AtomicInteger observedOnExit = new AtomicInteger(-1); + AtomicInteger activeCallbacks = new AtomicInteger(0); + + OutboundCallback callback = (userData, data, len) -> { + observedOnEntry.set(activeCallbacks.incrementAndGet()); + enteredLatch.countDown(); + try { + data.getByteArray(0, len); // copy as required + } finally { + observedOnExit.set(activeCallbacks.decrementAndGet()); + exitLatch.countDown(); + } + }; + + lib.connection_open(serverHandle, callback, Pointer.NULL, burstCount); + + assertTrue(enteredLatch.await(10, TimeUnit.SECONDS), "Callback must be entered within 10 seconds"); + assertEquals(1, observedOnEntry.get(), "Active count must be 1 while callback is executing"); + + assertTrue(exitLatch.await(10, TimeUnit.SECONDS), "Callback must exit within 10 seconds"); + assertEquals(0, observedOnExit.get(), "Active count must return to 0 after callback exits"); + } + + @Test + void callbackDataContainsJsonRpcContent() throws Exception { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + + CountDownLatch latch = new CountDownLatch(1); + AtomicReference receivedMessage = new AtomicReference<>(); + + OutboundCallback callback = (userData, data, len) -> { + try { + byte[] bytes = data.getByteArray(0, len); + receivedMessage.set(new String(bytes, StandardCharsets.UTF_8)); + } finally { + latch.countDown(); + } + }; + + lib.connection_open(serverHandle, callback, Pointer.NULL, 1); + + assertTrue(latch.await(10, TimeUnit.SECONDS), "Callback must complete within 10 seconds"); + String msg = receivedMessage.get(); + assertNotNull(msg, "Received message must not be null"); + assertTrue(msg.contains("jsonrpc"), "Callback data should contain JSON-RPC content, got: " + msg); + } + + @Test + void multipleCallbacksDoNotLeakActiveCount() throws Exception { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + + int burstCount = 5; + CountDownLatch latch = new CountDownLatch(burstCount); + AtomicInteger activeCallbacks = new AtomicInteger(0); + AtomicInteger maxObservedActive = new AtomicInteger(0); + + OutboundCallback callback = (userData, data, len) -> { + int current = activeCallbacks.incrementAndGet(); + maxObservedActive.updateAndGet(prev -> Math.max(prev, current)); + try { + data.getByteArray(0, len); + } finally { + activeCallbacks.decrementAndGet(); + latch.countDown(); + } + }; + + lib.connection_open(serverHandle, callback, Pointer.NULL, burstCount); + + assertTrue(latch.await(10, TimeUnit.SECONDS), "All callbacks must complete within timeout"); + assertEquals(0, activeCallbacks.get(), "Active count must be 0 after all callbacks complete"); + assertTrue(maxObservedActive.get() >= 1, "At least one callback must have been observed as active"); + } + + @Test + void connectionWriteReturnsTrueForValidHandle() { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + int connHandle = lib.connection_open(serverHandle, (ud, data, len) -> { + }, Pointer.NULL, 0); + + byte[] payload = "{\"jsonrpc\":\"2.0\",\"method\":\"ping\"}".getBytes(StandardCharsets.UTF_8); + assertTrue(lib.connection_write(connHandle, payload, payload.length), + "connection_write should return true for valid data"); + } + + @Test + void connectionCloseReturnsTrueForValidHandle() { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + int connHandle = lib.connection_open(serverHandle, (ud, data, len) -> { + }, Pointer.NULL, 0); + + assertTrue(lib.connection_close(connHandle), "connection_close should return true"); + } + + @Test + void hostShutdownReturnsTrueForValidHandle() { + if (!testLibExists()) { + return; + } + CallbackTestLib lib = loadTestLib(); + int serverHandle = lib.host_start(); + assertTrue(lib.host_shutdown(serverHandle), "host_shutdown should return true"); + } +} From 3b60f849f99f2f370fa47d389f02a97ef239a5a6 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Mon, 3 Aug 2026 16:59:53 +0000 Subject: [PATCH 3/3] fix: address Copilot code review findings for JNA binding - Fix callback GC: store tracked callbacks per connection in a ConcurrentHashMap, removed on connectionClose (comment #3706039757) - Fix boolean ABI mismatch: Rust bool is 1 byte, JNA maps Java boolean as 32-bit int. Changed CopilotRuntimeLibrary to return byte, convert to boolean in delegation methods (comment #3706039823) - Wrap UnsatisfiedLinkError in IllegalStateException per error contract (comment #3706039855) - Replace silent return with assumeTrue for native lib tests so skips are visible in CI reports (comment #3706039896) - Rewrite activeCallbackCount test to exercise through JnaNativeBinding and assert binding.activeCallbacks (comment #3706039935) - Add ABI name documentation to CallbackTestLib, fix byte return types in test interface (comment #3706039968) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../github/copilot/ffi/JnaNativeBinding.java | 62 ++++++-- .../copilot/ffi/JnaNativeBindingTest.java | 148 ++++++++---------- 2 files changed, 111 insertions(+), 99 deletions(-) diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java b/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java index 70536910a6..af3930bc99 100644 --- a/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java +++ b/java/sdk/src/main/java/com/github/copilot/ffi/JnaNativeBinding.java @@ -9,6 +9,8 @@ import com.sun.jna.Pointer; import java.nio.file.Path; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Logger; @@ -58,18 +60,33 @@ interface CopilotRuntimeLibrary extends Library { /** Corresponds to {@code copilot_runtime_host_start}. */ int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen); - /** Corresponds to {@code copilot_runtime_host_shutdown}. */ - boolean copilot_runtime_host_shutdown(int serverId); + /** + * Corresponds to {@code copilot_runtime_host_shutdown}. + * + *

+ * Returns {@code byte} (not Java {@code boolean}) because the Rust ABI exports + * a one-byte {@code bool}. JNA maps Java {@code boolean} as a 32-bit C + * {@code int}, which would read three extra bytes. + */ + byte copilot_runtime_host_shutdown(int serverId); /** Corresponds to {@code copilot_runtime_connection_open}. */ int copilot_runtime_connection_open(int serverId, OutboundCallback callback, Pointer userData, byte[] extSource, int extSourceLen, byte[] extName, int extNameLen, byte[] connToken, int connTokenLen); - /** Corresponds to {@code copilot_runtime_connection_write}. */ - boolean copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen); - - /** Corresponds to {@code copilot_runtime_connection_close}. */ - boolean copilot_runtime_connection_close(int connectionId); + /** + * Corresponds to {@code copilot_runtime_connection_write}. + * + * @see #copilot_runtime_host_shutdown for why this returns {@code byte} + */ + byte copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen); + + /** + * Corresponds to {@code copilot_runtime_connection_close}. + * + * @see #copilot_runtime_host_shutdown for why this returns {@code byte} + */ + byte copilot_runtime_connection_close(int connectionId); } // ------------------------------------------------------------------------- @@ -105,6 +122,12 @@ int copilot_runtime_connection_open(int serverId, OutboundCallback callback, Poi */ final AtomicInteger activeCallbacks = new AtomicInteger(0); + /** + * Tracked callback wrappers keyed by connection handle. Prevents GC of the JNA + * callback function pointer while native code still holds it. + */ + private final Map trackedCallbacks = new ConcurrentHashMap<>(); + // ------------------------------------------------------------------------- // Constructors // ------------------------------------------------------------------------- @@ -123,7 +146,11 @@ int copilot_runtime_connection_open(int serverId, OutboundCallback callback, Poi synchronized (LOAD_LOCK) { if (loadedLib == null) { LOG.fine(() -> "Loading native library from: " + absPath); - loadedLib = Native.load(absPath.toString(), CopilotRuntimeLibrary.class); + try { + loadedLib = Native.load(absPath.toString(), CopilotRuntimeLibrary.class); + } catch (UnsatisfiedLinkError e) { + throw new IllegalStateException("Failed to load native library from '" + absPath + "'", e); + } loadedPath = absPath; LOG.fine(() -> "Native library loaded: " + absPath); } else if (!absPath.equals(loadedPath)) { @@ -161,7 +188,7 @@ public int hostStart(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJs @Override public boolean hostShutdown(int serverId) { - return lib.copilot_runtime_host_shutdown(serverId); + return lib.copilot_runtime_host_shutdown(serverId) != 0; } @Override @@ -176,18 +203,27 @@ public int connectionOpen(int serverId, OutboundCallback callback, Pointer userD activeCallbacks.decrementAndGet(); } }; - return lib.copilot_runtime_connection_open(serverId, tracked, userData, extSource, extSourceLen, extName, - extNameLen, connToken, connTokenLen); + int connectionId = lib.copilot_runtime_connection_open(serverId, tracked, userData, extSource, extSourceLen, + extName, extNameLen, connToken, connTokenLen); + if (connectionId != 0) { + // Hold a strong reference to prevent GC of the JNA function pointer. + trackedCallbacks.put(connectionId, tracked); + } + return connectionId; } @Override public boolean connectionWrite(int connectionId, byte[] data, int dataLen) { - return lib.copilot_runtime_connection_write(connectionId, data, dataLen); + return lib.copilot_runtime_connection_write(connectionId, data, dataLen) != 0; } @Override public boolean connectionClose(int connectionId) { - return lib.copilot_runtime_connection_close(connectionId); + try { + return lib.copilot_runtime_connection_close(connectionId) != 0; + } finally { + trackedCallbacks.remove(connectionId); + } } // ------------------------------------------------------------------------- diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java b/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java index 790c2607f7..9f0f939eee 100644 --- a/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java +++ b/java/sdk/src/test/java/com/github/copilot/ffi/JnaNativeBindingTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; import com.sun.jna.Library; import com.sun.jna.Native; @@ -64,15 +65,23 @@ class JnaNativeBindingTest { /** * JNA interface for the simplified test library. Maps Java names to the * snake_case exports of {@code libcallback_test}. + * + *

+ * Note: This test library exports simplified names ({@code host_start}, + * {@code connection_open}, etc.) rather than the production + * {@code copilot_runtime_*} names. Production symbol resolution is validated + * lazily by JNA when each method is first called through + * {@link JnaNativeBinding.CopilotRuntimeLibrary}. */ interface CallbackTestLib extends Library { /** Simulates {@code copilot_runtime_host_start}; always returns 42. */ int host_start(); /** - * Simulates {@code copilot_runtime_host_shutdown}; always returns {@code true}. + * Simulates {@code copilot_runtime_host_shutdown}; always returns nonzero. + * Returns {@code byte} to match the Rust ABI one-byte {@code bool}. */ - boolean host_shutdown(int serverHandle); + byte host_shutdown(int serverHandle); /** * Simulates {@code copilot_runtime_connection_open}. Spawns a native thread @@ -81,16 +90,16 @@ interface CallbackTestLib extends Library { int connection_open(int serverHandle, OutboundCallback callback, Pointer userData, int burstCount); /** - * Simulates {@code copilot_runtime_connection_write}; always returns - * {@code true}. + * Simulates {@code copilot_runtime_connection_write}; always returns nonzero. + * Returns {@code byte} to match the Rust ABI one-byte {@code bool}. */ - boolean connection_write(int connectionHandle, byte[] data, int len); + byte connection_write(int connectionHandle, byte[] data, int len); /** - * Simulates {@code copilot_runtime_connection_close}; always returns - * {@code true}. + * Simulates {@code copilot_runtime_connection_close}; always returns nonzero. + * Returns {@code byte} to match the Rust ABI one-byte {@code bool}. */ - boolean connection_close(int connectionHandle); + byte connection_close(int connectionHandle); } // ------------------------------------------------------------------------- @@ -103,15 +112,16 @@ interface CallbackTestLib extends Library { */ private static class StubRuntimeLibrary implements JnaNativeBinding.CopilotRuntimeLibrary { int hostStartReturn = 1; - boolean hostShutdownReturn = true; + byte hostShutdownReturn = 1; int connectionOpenReturn = 1; - boolean connectionWriteReturn = true; - boolean connectionCloseReturn = true; + byte connectionWriteReturn = 1; + byte connectionCloseReturn = 1; byte[] lastArgvJson; int lastArgvJsonLen; int lastServerId; int lastConnectionId; + OutboundCallback lastCallback; @Override public int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, byte[] envJson, int envJsonLen) { @@ -121,7 +131,7 @@ public int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, byte[] e } @Override - public boolean copilot_runtime_host_shutdown(int serverId) { + public byte copilot_runtime_host_shutdown(int serverId) { lastServerId = serverId; return hostShutdownReturn; } @@ -131,17 +141,18 @@ public int copilot_runtime_connection_open(int serverId, OutboundCallback callba byte[] extSource, int extSourceLen, byte[] extName, int extNameLen, byte[] connToken, int connTokenLen) { lastServerId = serverId; + lastCallback = callback; return connectionOpenReturn; } @Override - public boolean copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen) { + public byte copilot_runtime_connection_write(int connectionId, byte[] data, int dataLen) { lastConnectionId = connectionId; return connectionWriteReturn; } @Override - public boolean copilot_runtime_connection_close(int connectionId) { + public byte copilot_runtime_connection_close(int connectionId) { lastConnectionId = connectionId; return connectionCloseReturn; } @@ -199,7 +210,7 @@ void hostStartReturnsZeroOnFailure() { @Test void hostShutdownDelegatesToLibrary() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.hostShutdownReturn = true; + stub.hostShutdownReturn = 1; JnaNativeBinding binding = new JnaNativeBinding(stub); assertTrue(binding.hostShutdown(42)); @@ -209,7 +220,7 @@ void hostShutdownDelegatesToLibrary() { @Test void hostShutdownReturnsFalseOnFailure() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.hostShutdownReturn = false; + stub.hostShutdownReturn = 0; JnaNativeBinding binding = new JnaNativeBinding(stub); assertFalse(binding.hostShutdown(1)); } @@ -243,7 +254,7 @@ void connectionOpenReturnsZeroOnFailure() { @Test void connectionWriteDelegatesToLibrary() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.connectionWriteReturn = true; + stub.connectionWriteReturn = 1; JnaNativeBinding binding = new JnaNativeBinding(stub); byte[] data = "hello".getBytes(StandardCharsets.UTF_8); @@ -254,7 +265,7 @@ void connectionWriteDelegatesToLibrary() { @Test void connectionWriteReturnsFalseOnFailure() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.connectionWriteReturn = false; + stub.connectionWriteReturn = 0; JnaNativeBinding binding = new JnaNativeBinding(stub); byte[] data = "x".getBytes(StandardCharsets.UTF_8); @@ -265,7 +276,7 @@ void connectionWriteReturnsFalseOnFailure() { @Test void connectionCloseDelegatesToLibrary() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.connectionCloseReturn = true; + stub.connectionCloseReturn = 1; JnaNativeBinding binding = new JnaNativeBinding(stub); assertTrue(binding.connectionClose(7)); assertEquals(7, stub.lastConnectionId); @@ -274,7 +285,7 @@ void connectionCloseDelegatesToLibrary() { @Test void connectionCloseReturnsFalseOnFailure() { StubRuntimeLibrary stub = new StubRuntimeLibrary(); - stub.connectionCloseReturn = false; + stub.connectionCloseReturn = 0; JnaNativeBinding binding = new JnaNativeBinding(stub); assertFalse(binding.connectionClose(1)); } @@ -292,18 +303,14 @@ void activeCallbacksStartsAtZero() { @Test void loadByPathSucceedsWhenLibraryExists() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); JnaNativeBinding binding = new JnaNativeBinding(testLibAbsPath()); assertNotNull(binding); } @Test void loadByPathTwiceWithSamePathSucceeds() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); new JnaNativeBinding(testLibAbsPath()); // Second construction with the same absolute path must not throw. new JnaNativeBinding(testLibAbsPath()); @@ -311,9 +318,7 @@ void loadByPathTwiceWithSamePathSucceeds() { @Test void activeCallbacksStartsAtZeroAfterPathLoad() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); JnaNativeBinding binding = new JnaNativeBinding(testLibAbsPath()); assertEquals(0, binding.activeCallbacks.get()); } @@ -324,9 +329,7 @@ void activeCallbacksStartsAtZeroAfterPathLoad() { @Test void loadFromDifferentPathThrowsIllegalState(@TempDir Path tempDir) throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); Path altPath = tempDir.resolve("libcallback_test_alt.so"); Files.copy(testLibAbsPath(), altPath); @@ -342,9 +345,7 @@ void loadFromDifferentPathThrowsIllegalState(@TempDir Path tempDir) throws Excep @Test void duplicateLoadDiagnosticMentionsNotSupported(@TempDir Path tempDir) throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); Path altPath = tempDir.resolve("libcallback_test_b.so"); Files.copy(testLibAbsPath(), altPath); @@ -357,9 +358,7 @@ void duplicateLoadDiagnosticMentionsNotSupported(@TempDir Path tempDir) throws E @Test void resetForTestingAllowsReloadFromDifferentPath(@TempDir Path tempDir) throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); Path altPath = tempDir.resolve("libcallback_test_reset.so"); Files.copy(testLibAbsPath(), altPath); @@ -377,9 +376,7 @@ void resetForTestingAllowsReloadFromDifferentPath(@TempDir Path tempDir) throws @Test void callbackIsInvokedFromNativeThread() throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); assertEquals(42, serverHandle, "host_start should return 42"); @@ -412,45 +409,32 @@ void callbackIsInvokedFromNativeThread() throws Exception { } @Test - void activeCallbackCountIsIncrementedDuringCallback() throws Exception { - if (!testLibExists()) { - return; - } - CallbackTestLib lib = loadTestLib(); - int serverHandle = lib.host_start(); + void activeCallbackCountIsIncrementedDuringCallback() { + StubRuntimeLibrary stub = new StubRuntimeLibrary(); + stub.connectionOpenReturn = 99; + JnaNativeBinding binding = new JnaNativeBinding(stub); - int burstCount = 1; - CountDownLatch enteredLatch = new CountDownLatch(burstCount); - CountDownLatch exitLatch = new CountDownLatch(burstCount); - AtomicInteger observedOnEntry = new AtomicInteger(-1); - AtomicInteger observedOnExit = new AtomicInteger(-1); - AtomicInteger activeCallbacks = new AtomicInteger(0); + AtomicInteger observedDuringCallback = new AtomicInteger(-1); - OutboundCallback callback = (userData, data, len) -> { - observedOnEntry.set(activeCallbacks.incrementAndGet()); - enteredLatch.countDown(); - try { - data.getByteArray(0, len); // copy as required - } finally { - observedOnExit.set(activeCallbacks.decrementAndGet()); - exitLatch.countDown(); - } + OutboundCallback userCallback = (userData, data, len) -> { + // Observe binding.activeCallbacks while inside the callback + observedDuringCallback.set(binding.activeCallbacks.get()); }; - lib.connection_open(serverHandle, callback, Pointer.NULL, burstCount); + binding.connectionOpen(1, userCallback, Pointer.NULL, null, 0, null, 0, null, 0); - assertTrue(enteredLatch.await(10, TimeUnit.SECONDS), "Callback must be entered within 10 seconds"); - assertEquals(1, observedOnEntry.get(), "Active count must be 1 while callback is executing"); + // The stub captured the tracked wrapper — invoke it to trigger tracking + assertNotNull(stub.lastCallback, "Stub must have captured the tracked callback"); + stub.lastCallback.invoke(Pointer.NULL, Pointer.NULL, 0); - assertTrue(exitLatch.await(10, TimeUnit.SECONDS), "Callback must exit within 10 seconds"); - assertEquals(0, observedOnExit.get(), "Active count must return to 0 after callback exits"); + assertEquals(1, observedDuringCallback.get(), "binding.activeCallbacks must be 1 during callback execution"); + assertEquals(0, binding.activeCallbacks.get(), + "binding.activeCallbacks must return to 0 after callback completes"); } @Test void callbackDataContainsJsonRpcContent() throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); @@ -476,9 +460,7 @@ void callbackDataContainsJsonRpcContent() throws Exception { @Test void multipleCallbacksDoNotLeakActiveCount() throws Exception { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); @@ -507,39 +489,33 @@ void multipleCallbacksDoNotLeakActiveCount() throws Exception { @Test void connectionWriteReturnsTrueForValidHandle() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); int connHandle = lib.connection_open(serverHandle, (ud, data, len) -> { }, Pointer.NULL, 0); byte[] payload = "{\"jsonrpc\":\"2.0\",\"method\":\"ping\"}".getBytes(StandardCharsets.UTF_8); - assertTrue(lib.connection_write(connHandle, payload, payload.length), - "connection_write should return true for valid data"); + assertTrue(lib.connection_write(connHandle, payload, payload.length) != 0, + "connection_write should return nonzero for valid data"); } @Test void connectionCloseReturnsTrueForValidHandle() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); int connHandle = lib.connection_open(serverHandle, (ud, data, len) -> { }, Pointer.NULL, 0); - assertTrue(lib.connection_close(connHandle), "connection_close should return true"); + assertTrue(lib.connection_close(connHandle) != 0, "connection_close should return nonzero"); } @Test void hostShutdownReturnsTrueForValidHandle() { - if (!testLibExists()) { - return; - } + assumeTrue(testLibExists(), "Native test library not found at " + SPIKE_LIB_PATH); CallbackTestLib lib = loadTestLib(); int serverHandle = lib.host_start(); - assertTrue(lib.host_shutdown(serverHandle), "host_shutdown should return true"); + assertTrue(lib.host_shutdown(serverHandle) != 0, "host_shutdown should return nonzero"); } }