diff --git a/javatools/src/main/java/org/xvm/runtime/ServiceContext.java b/javatools/src/main/java/org/xvm/runtime/ServiceContext.java index a8e59ae058..847e86b33a 100644 --- a/javatools/src/main/java/org/xvm/runtime/ServiceContext.java +++ b/javatools/src/main/java/org/xvm/runtime/ServiceContext.java @@ -6,7 +6,6 @@ import java.util.Arrays; import java.util.EnumMap; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Queue; @@ -273,19 +272,8 @@ private Map ensureTransientMap() { * * @return the map of callbacks keyed by unique ids */ - protected Map ensureCallbackMap() { - Map map = m_mapCallbacks; - if (map == null) { - map = m_mapCallbacks = new HashMap<>(); - } - return map; - } - - /** - * @return the map of callbacks keyed by unique ids - */ protected Map getCallbackMap() { - return m_mapCallbacks; + return f_mapCallbacks; } // ----- scheduling --------------------------------------------------------------------------- @@ -1030,7 +1018,7 @@ public CompletableFuture callLater(FunctionHandle hFunction, Objec if (future != null) { future.whenComplete((r, x) -> { if (x != null) { - callUnhandledExceptionHandler(((WrapperException) x).getExceptionHandle()); + reportUnhandledException(x); } }); } @@ -1053,7 +1041,7 @@ public CompletableFuture callLater(Frame frame, FunctionHandle hFu if (future != null) { future.whenComplete((r, x) -> { if (x != null) { - callUnhandledExceptionHandler(((WrapperException) x).getExceptionHandle()); + reportUnhandledException(x); } }); } @@ -1505,6 +1493,34 @@ public String toString() { return request.f_future; } + /** + * Report a failed "callLater" to the unhandled exception handler. + *

+ * This runs inside a {@link CompletableFuture} completion stage, which discards anything thrown + * out of it - so nothing here may assume what the failure is, and nothing here may throw. The + * previous code cast the throwable straight to {@link WrapperException}: any other failure + * turned into a ClassCastException that the completion stage swallowed, taking the original + * failure with it and leaving the handler uncalled. A reported failure became a silent one. + * + * @param e the throwable the future completed with; never null + */ + private void reportUnhandledException(Throwable e) { + try { + // translate() unwraps CompletionException/ExecutionException, maps a WrapperException + // to its handle, and renders anything else - including a cancellation, an interrupt, or + // a native failure - as a visible exception rather than discarding it + ExceptionHandle hException = Utils.translate(e); + if (hException != null) { + callUnhandledExceptionHandler(hException); + } + } catch (Throwable eReport) { + // must not happen, and must not escape into the completion stage that would swallow it + System.err.println("Unexpected failure reporting an unhandled exception: " + f_sName); + eReport.printStackTrace(System.err); + e.printStackTrace(System.err); + } + } + protected void callUnhandledExceptionHandler(ExceptionHandle hException) { FunctionHandle hFunction = m_hExceptionHandler; if (hFunction == null) { @@ -2191,9 +2207,12 @@ public enum ServiceStatus { private Map m_mapTransient; /** - * A "service-local" cache for service callbacks. + * The registered service callbacks. The map is deliberately eager, final, and concurrent: the + * owning service registers callbacks on its own thread, but alarm maturation extracts them on + * the shared native timer thread and alarm cancellation discards them from natural code, so + * this registry is mutated from multiple threads by design. */ - private Map m_mapCallbacks; + private final Map f_mapCallbacks = new ConcurrentHashMap<>(); /** * A wake-up scheduler to process registered timeouts. diff --git a/javatools/src/main/java/org/xvm/runtime/WeakCallback.java b/javatools/src/main/java/org/xvm/runtime/WeakCallback.java index fcdee7bbf2..22f164264a 100644 --- a/javatools/src/main/java/org/xvm/runtime/WeakCallback.java +++ b/javatools/src/main/java/org/xvm/runtime/WeakCallback.java @@ -18,21 +18,36 @@ public WeakCallback(Frame frame, FunctionHandle hFunction) { super(frame.f_context); f_lCallbackId = frame.f_context.f_container.f_runtime.makeUniqueId(); - frame.f_context.ensureCallbackMap().put(f_lCallbackId, new Callback(frame, hFunction)); + frame.f_context.getCallbackMap().put(f_lCallbackId, new Callback(frame, hFunction)); } /** - * @return the underlying function; never null + * Extract the callback data, removing it from the owning service's registry. + *

+ * This runs on the shared native timer thread. A missing callback is a normal outcome - the + * service may have been collected, or the alarm may have been discarded by a racing cancel - + * and must never throw here: an exception escaping a {@link java.util.TimerTask} kills the + * shared static {@link java.util.Timer} and silently disables every alarm in every container. + * + * @return the callback data, or null if the service is gone or the callback was already + * extracted or discarded */ public Callback extractCallback() { + ServiceContext context = get(); + return context == null + ? null + : context.getCallbackMap().remove(f_lCallbackId); + } + + /** + * Discard the callback data without running it. Called when an alarm is canceled, so that the + * registry does not leak the captured frame and function for the lifetime of the service. + */ + public void discard() { ServiceContext context = get(); if (context != null) { - Callback callback = context.getCallbackMap().remove(f_lCallbackId); - if (callback != null) { - return callback; - } + context.getCallbackMap().remove(f_lCallbackId); } - throw new IllegalStateException(); } @Override diff --git a/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xLocalClock.java b/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xLocalClock.java index 95779f1d2b..4878ea2167 100644 --- a/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xLocalClock.java +++ b/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xLocalClock.java @@ -128,9 +128,10 @@ private int invokeSchedule(Frame frame, long ldtWakeup, long cDelay, Alarm alarm = new Alarm(new WeakCallback(frame, hAlarm), ldtWakeup, hKeepAlive.get()); try { + alarm.registerKeepAlive(); TIMER.schedule(alarm.getTrigger(), cDelay); } catch (Exception e) { - alarm.cancel(); + alarm.cancelAfterScheduleFailure(); return frame.raiseException(e.getMessage()); } @@ -200,11 +201,7 @@ protected Alarm(WeakCallback refCallback, long ldtWakeUp, boolean fKeepAlive) { f_refCallback = refCallback; f_ldtWakeup = ldtWakeUp; m_trigger = new Trigger(this); - f_Registered = fKeepAlive; - - if (fKeepAlive) { - refCallback.get().f_container.registerNativeCallback(); - } + f_fKeepAlive = fKeepAlive; } /** @@ -214,6 +211,35 @@ public Trigger getTrigger() { return m_trigger; } + /** + * Claim container keep-alive ownership as part of the schedule attempt rather than from the + * constructor. Registering in the constructor left the count elevated forever when + * Timer.schedule(...) failed, since TimerTask.cancel() reports false for a task that was + * never scheduled and the old cancel() path therefore skipped the unregister. + */ + public void registerKeepAlive() { + if (!f_fKeepAlive) { + return; + } + + ServiceContext context = f_refCallback.get(); + if (context == null) { + return; + } + + Container container = context.f_container; + synchronized (this) { + if (m_fFinished || m_containerRegistered != null) { + return; + } + + // remember the exact owner while the callback is registered: the WeakCallback can + // legitimately clear before cleanup, but keep-alive ownership must still unwind + container.registerNativeCallback(); + m_containerRegistered = container; + } + } + /** * Called when the alarm is triggered by the Java timer. */ @@ -231,15 +257,23 @@ public void run() { long ldtNow = container.currentTimeMillis(); if (ldtNow >= f_ldtWakeup) { - WeakCallback.Callback callback = f_refCallback.extractCallback(); - context.callLater(callback.frame(), callback.functionHandle(), Utils.OBJECTS_NONE); - if (f_Registered) { - container.unregisterNativeCallback(); + Container containerRegistered = finish(); + try { + WeakCallback.Callback callback = f_refCallback.extractCallback(); + if (callback != null) { + context.callLater(callback.frame(), callback.functionHandle(), + Utils.OBJECTS_NONE); + } + } finally { + unregister(containerRegistered); } } else { // reschedule TIMER.schedule(m_trigger = new Trigger(this), f_ldtWakeup - ldtNow); } + } else { + // the owning service is gone; release the keep-alive count we still hold + unregister(finish()); } } @@ -247,14 +281,46 @@ public void run() { * Called when the alarm is canceled by the natural code. */ public boolean cancel() { - boolean fCancelled = m_trigger.cancel(); - ServiceContext context = f_refCallback.get(); - if (context != null && fCancelled && f_Registered) { - context.f_container.unregisterNativeCallback(); + boolean fCancelled = m_trigger.cancel(); + if (fCancelled) { + // the trigger will never run, so the registry entry would otherwise leak the + // captured frame and function for the lifetime of the service + f_refCallback.discard(); + unregister(finish()); } return fCancelled; } + /** + * Roll back a failed schedule attempt. The alarm has not escaped to natural code yet, so it + * can be marked finished even though TimerTask.cancel() reports false for a task that was + * never scheduled. + */ + public void cancelAfterScheduleFailure() { + m_trigger.cancel(); + f_refCallback.discard(); + unregister(finish()); + } + + /** + * Mark this alarm done and hand off the keep-alive ownership, if any, exactly once. + * + * @return the container this alarm registered a keep-alive callback with, or null + */ + private synchronized Container finish() { + m_fFinished = true; + + Container container = m_containerRegistered; + m_containerRegistered = null; + return container; + } + + private static void unregister(Container container) { + if (container != null) { + container.unregisterNativeCallback(); + } + } + /** * A TimerTask that is scheduled on the Java timer and is used to trigger the alarm. */ @@ -274,8 +340,10 @@ public void run() { private final WeakCallback f_refCallback; private final long f_ldtWakeup; - private final boolean f_Registered; + private final boolean f_fKeepAlive; private Trigger m_trigger; + private Container m_containerRegistered; + private boolean m_fFinished; } // ----- constants and fields ------------------------------------------------------------------ diff --git a/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xNanosTimer.java b/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xNanosTimer.java index a2ad0daf1b..56027454e3 100644 --- a/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xNanosTimer.java +++ b/javatools/src/main/java/org/xvm/runtime/template/_native/temporal/xNanosTimer.java @@ -143,8 +143,17 @@ private int invokeSchedule(Frame frame, TimerHandle hTimer, GenericHandle hDurat LongLongHandle llPicos = (LongLongHandle) hDuration.getField(frame, "picoseconds"); long cNanos = Math.max(0, llPicos.getValue().divUnsigned(PICOS_PER_NANO).getLowValue()); - return frame.assignValue(iReturn, - hTimer.addAlarm(cNanos, new WeakCallback(frame, hAlarm), hKeepAlive.get())); + WeakCallback refCallback = new WeakCallback(frame, hAlarm); + try { + return frame.assignValue(iReturn, + hTimer.addAlarm(cNanos, refCallback, hKeepAlive.get())); + } catch (IllegalArgumentException | IllegalStateException e) { + // the JVM scheduler refused the task; drop the registry entry we just created and + // report the failure to natural code instead of silently returning an alarm that will + // never fire + refCallback.discard(); + return frame.raiseException(e.getMessage()); + } } // ----- ObjectHandle -------------------------------------------------------------------------- @@ -258,8 +267,15 @@ public FunctionHandle addAlarm(long cNanos, WeakCallback refCallback, boolean fK f_setAlarms.add(alarm); } - if (isRunning()) { - alarm.start(); + try { + if (isRunning()) { + alarm.start(); + } + } catch (RuntimeException | Error e) { + synchronized (f_setAlarms) { + f_setAlarms.remove(alarm); + } + throw e; } return new NativeFunctionHandle((_frame, _ah, _iReturn) -> { @@ -291,7 +307,7 @@ public class Alarm { public Alarm(long cNanosDelay, WeakCallback refCallback, boolean fKeepAlive) { f_cNanosAlarm = cNanosDelay + refCallback.get().f_container.nanoTime(); f_refCallback = refCallback; - f_fRegistered = fKeepAlive; + f_fKeepAlive = fKeepAlive; } /** @@ -307,14 +323,16 @@ public synchronized void start() { Trigger trigger = createTrigger(); try { - if (f_fRegistered) { - container.registerNativeCallback(); - } + registerKeepAlive(container); long cDelay = (f_cNanosAlarm - m_cNanosStart - m_cNanosBurnt) / NANOS_PER_MILLI; xLocalClock.TIMER.schedule(trigger, Math.max(1, cDelay)); - } catch (Throwable e) { + } catch (RuntimeException | Error e) { + // the old code swallowed this, handing natural code an alarm that would never + // fire while the keep-alive count stayed elevated forever cancelTrigger(); + unregisterKeepAlive(); + throw e; } } @@ -370,25 +388,26 @@ public void run() { m_trigger = null; } - ServiceContext context = f_refCallback.get(); - if (context != null) { - WeakCallback.Callback callback = f_refCallback.extractCallback(); - context.callLater(callback.frame(), callback.functionHandle(), Utils.OBJECTS_NONE); - if (f_fRegistered) { - context.f_container.unregisterNativeCallback(); + try { + ServiceContext context = f_refCallback.get(); + if (context != null) { + WeakCallback.Callback callback = f_refCallback.extractCallback(); + if (callback != null) { + context.callLater(callback.frame(), callback.functionHandle(), + Utils.OBJECTS_NONE); + } } + } finally { + unregisterKeepAlive(); + TimerHandle.this.removeAlarm(this); } - TimerHandle.this.removeAlarm(this); } /** * Called after alarm has finished or has been canceled. */ public void unregister() { - ServiceContext context = f_refCallback.get(); - if (context != null && f_fRegistered) { - context.f_container.unregisterNativeCallback(); - } + unregisterKeepAlive(); } /** @@ -402,6 +421,12 @@ public void cancel() { m_fDead = true; cancelTrigger(); + + // m_fDead guarantees run() has not fired and never will, so the registry entry + // would otherwise leak the captured frame and function for the lifetime of the + // service + f_refCallback.discard(); + unregisterKeepAlive(); } TimerHandle.this.removeAlarm(this); @@ -418,6 +443,29 @@ protected void cancelTrigger() { } } + /** + * Claim container keep-alive ownership for this alarm, remembering the exact owner. + * The WeakCallback can legitimately clear before cleanup runs, but keep-alive ownership + * must still unwind against the container it was taken from. + */ + private synchronized void registerKeepAlive(Container container) { + if (f_fKeepAlive && m_containerRegistered == null) { + container.registerNativeCallback(); + m_containerRegistered = container; + } + } + + /** + * Release container keep-alive ownership, if this alarm still holds it. Idempotent. + */ + private synchronized void unregisterKeepAlive() { + Container container = m_containerRegistered; + if (container != null) { + m_containerRegistered = null; + container.unregisterNativeCallback(); + } + } + /** * @return zero if the alarm is already up, or time interval in milliseconds until it's * going to be up @@ -476,9 +524,10 @@ public boolean cancel() { private final WeakCallback f_refCallback; private final long f_cNanosAlarm; - private final boolean f_fRegistered; + private final boolean f_fKeepAlive; private long m_cNanosStart; private long m_cNanosBurnt; + private Container m_containerRegistered; private volatile boolean m_fDead; private volatile Trigger m_trigger; } diff --git a/javatools/src/main/java/org/xvm/runtime/template/_native/web/xRTServer.java b/javatools/src/main/java/org/xvm/runtime/template/_native/web/xRTServer.java index 2f32f97c78..3b276bb355 100644 --- a/javatools/src/main/java/org/xvm/runtime/template/_native/web/xRTServer.java +++ b/javatools/src/main/java/org/xvm/runtime/template/_native/web/xRTServer.java @@ -30,7 +30,6 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; @@ -249,6 +248,8 @@ private int invokeBind(Frame frame, HttpServerHandle hServer, ObjectHandle[] ahA int nHttpPort = (int) ((JavaLong) ahArg[2]).getValue(); int nHttpsPort = (int) ((JavaLong) ahArg[3]).getValue(); + ExecutorService executor = null; + boolean fCallbackRegistered = false; try { configureHttpServer (hServer, new InetSocketAddress(sBindAddr, nHttpPort)); configureHttpsServer(hServer, new InetSocketAddress(sBindAddr, nHttpsPort)); @@ -272,7 +273,7 @@ private int invokeBind(Frame frame, HttpServerHandle hServer, ObjectHandle[] ahA // (see HttpHandler.x in xenia.xtclang.org module). // If necessary, we can change the start() method to take an array of handlers and // demultiplex it earlier by the native code - Executor executor = Executors.newCachedThreadPool(factory); + executor = Executors.newCachedThreadPool(factory); httpServer.setExecutor(executor); httpServer.start(); @@ -282,6 +283,7 @@ private int invokeBind(Frame frame, HttpServerHandle hServer, ObjectHandle[] ahA // prevent the container from being terminated hServer.f_context.f_container.registerNativeCallback(); + fCallbackRegistered = true; Router router = hServer.getRouter(); httpServer .createContext("/", router); @@ -289,6 +291,7 @@ private int invokeBind(Frame frame, HttpServerHandle hServer, ObjectHandle[] ahA return Op.R_NEXT; } catch (Exception e) { + rollbackBind(hServer, executor, fCallbackRegistered); frame.f_context.f_container.terminate(hServer.f_context); return frame.raiseException(xException.obscureIoException(frame, e.getMessage())); } @@ -330,7 +333,52 @@ public void configure(HttpsParameters params) { private void configureBinding(HttpServerHandle hServer, ObjectHandle hBinding) { hServer.setBinding(hBinding); -} + } + + /** + * Undo everything a failed {@link #invokeBind} attempt may have claimed. Without this, a + * failure after the keep-alive registration pinned the container's native callback count + * forever - preventing idle termination and hanging "join()" - and left partially configured + * Java servers and their thread pool behind. + */ + private static void rollbackBind( + HttpServerHandle hServer, ExecutorService executor, boolean fCallbackRegistered) { + if (fCallbackRegistered) { + hServer.f_context.f_container.unregisterNativeCallback(); + } + + closeServerQuietly(hServer.getHttpServer()); + closeServerQuietly(hServer.getHttpsServer()); + + if (executor != null) { + executor.shutdown(); + } + + Router router = hServer.getRouter(); + if (router != null) { + router.mapRoutes.clear(); + } + hServer.clear(); + } + + private static void closeServerQuietly(HttpServer server) { + if (server == null) { + return; + } + + try { + // a server that never got an executor was never started, and com.sun's HttpServer only + // releases such a socket after a start/stop pair + if (server.getExecutor() == null) { + server.start(); + } + server.stop(0); + } catch (RuntimeException _) { + // deliberately swallowed: the bind failure that triggered the rollback is the error + // natural code needs to see, not whatever this half-configured server says on the way + // out + } + } /** * Implementation of "void addRouteImpl(String hostName, UInt16 httpPort, UInt16 httpsPort, diff --git a/javatools/src/test/java/org/xvm/runtime/CallLaterFailureReportingTest.java b/javatools/src/test/java/org/xvm/runtime/CallLaterFailureReportingTest.java new file mode 100644 index 0000000000..3f8b33f098 --- /dev/null +++ b/javatools/src/test/java/org/xvm/runtime/CallLaterFailureReportingTest.java @@ -0,0 +1,127 @@ +package org.xvm.runtime; + + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.junit.jupiter.api.Test; + +import org.xvm.asm.Op; + +import org.xvm.runtime.template.xService; + +import org.xvm.runtime.template._native.reflect.xRTFunction.FunctionHandle; +import org.xvm.runtime.template._native.reflect.xRTFunction.NativeFunctionHandle; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + + +/** + * "callLater" promises that any failure of the called function is reported as an + * UnhandledExceptionNotification. It reported the failure from inside a + * {@link CompletableFuture} completion stage, which discards whatever is thrown out of it, so any + * failure the reporting code could not handle was lost silently along with the original. + */ +public class CallLaterFailureReportingTest { + /** + * The reporting lambda cast the throwable straight to WrapperException. "callLater" hands its + * future to the caller, and cancelling a CompletableFuture completes it with + * CancellationException - so the cast raised ClassCastException inside the completion stage, + * which swallowed it. The handler never ran and the cancellation was never reported anywhere: + * a failure that the runtime is contractually required to surface just disappeared. + */ + @Test + public void aCancelledCallLaterIsStillReported() throws Exception { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), + "compiled XDK system modules are required"); + + ServiceContext context = liveService(); + var reported = recordUnhandledExceptions(context); + + CompletableFuture future = context.callLater(doNothing(), Utils.OBJECTS_NONE); + assertNotNull(future, "the service must accept the request"); + + future.cancel(true); + + assertTrue(await(reported).toString().contains("cancelled"), + "the reported failure must describe the original cancellation"); + } + + /** + * The ordinary path - the called function raises an XTC exception, which arrives as a + * WrapperException - must keep reporting the original exception handle unchanged. + */ + @Test + public void aFailedCallLaterReportsTheOriginalException() throws Exception { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), + "compiled XDK system modules are required"); + + ServiceContext context = liveService(); + var reported = recordUnhandledExceptions(context); + + FunctionHandle hRaise = new NativeFunctionHandle( + (frame, _ahArg, _iReturn) -> frame.raiseException(FAILURE_TEXT)); + + assertNotNull(context.callLater(hRaise, Utils.OBJECTS_NONE), + "the service must accept the request"); + + assertTrue(await(reported).toString().contains(FAILURE_TEXT), + "the handler must receive the exception the function actually raised"); + } + + + // ----- helpers ------------------------------------------------------------------------------- + + /** + * Create a service that is properly formed, i.e. one that owns a service handle. + *

+ * A context without a handle reports itself terminated as soon as it runs its first frame + * (see ServiceContext.isTerminated), and a terminated service silently drops posted requests - + * including the unhandled-exception notification this test is about. + */ + private static ServiceContext liveService() { + ServiceContext context = RuntimeTestSupport.newContainer() + .createServiceContext("callLater"); + + xService.INSTANCE.createServiceHandle(context, + xService.INSTANCE.getCanonicalClass(), xService.INSTANCE.getCanonicalType()); + return context; + } + + /** + * Install an unhandled-exception handler that captures what it is given. + */ + private static CompletableFuture recordUnhandledExceptions(ServiceContext context) { + var reported = new CompletableFuture(); + + context.m_hExceptionHandler = new NativeFunctionHandle((_frame, ahArg, _iReturn) -> { + reported.complete(ahArg[0]); + return Op.R_NEXT; + }); + return reported; + } + + private static FunctionHandle doNothing() { + return new NativeFunctionHandle((_frame, _ahArg, _iReturn) -> Op.R_NEXT); + } + + private static ObjectHandle await(CompletableFuture reported) throws Exception { + try { + return reported.get(REPORT_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (TimeoutException e) { + throw new AssertionError("a failed callLater must reach the unhandled exception " + + "handler, but the failure was discarded by the completion stage", e); + } + } + + + // ----- constants ----------------------------------------------------------------------------- + + private static final String FAILURE_TEXT = "deliberate callLater failure"; + + private static final long REPORT_TIMEOUT_SECONDS = 15L; +} diff --git a/javatools/src/test/java/org/xvm/runtime/NativeCallbackRegistrationTest.java b/javatools/src/test/java/org/xvm/runtime/NativeCallbackRegistrationTest.java new file mode 100644 index 0000000000..79e8b582c5 --- /dev/null +++ b/javatools/src/test/java/org/xvm/runtime/NativeCallbackRegistrationTest.java @@ -0,0 +1,236 @@ +package org.xvm.runtime; + + +import java.util.Timer; + +import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; + +import org.xvm.asm.Op; + +import org.xvm.runtime.ObjectHandle.GenericHandle; + +import org.xvm.runtime.template.xBoolean; + +import org.xvm.runtime.template.numbers.LongLong; +import org.xvm.runtime.template.numbers.xInt128; + +import org.xvm.runtime.template._native.temporal.xLocalClock; +import org.xvm.runtime.template._native.temporal.xNanosTimer; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + + +/** + * Behavioral tests for the native alarm callback registry and for keep-alive ownership on the + * alarm-scheduling failure path. These boot a real {@link NativeContainer} and drive the real + * runtime classes; nothing here inspects source text. + */ +public class NativeCallbackRegistrationTest { + /** + * The registry is shared between the service thread that registers callbacks and the + * process-wide Java timer thread that extracts them when an alarm matures, so it must be a live + * concurrent map from the moment the service exists. It used to be a plain HashMap published + * lazily through a non-final field, which meant it was null until someone happened to register + * the first callback, and unsynchronized forever after. + */ + @Test + public void callbackRegistryIsLiveBeforeAnyCallbackIsRegistered() { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), "compiled XDK system modules are required"); + + ServiceContext context = RuntimeTestSupport.newContainer().createServiceContext("registry"); + + assertNotNull(context.getCallbackMap(), + "the callback registry must exist before the first callback is registered"); + assertTrue(context.getCallbackMap().isEmpty(), + "a fresh service must start with an empty callback registry"); + } + + /** + * Extraction runs on the shared Java timer thread. A callback that is already gone - because + * the service was collected, or because a racing cancel discarded it - used to raise + * IllegalStateException there. An exception escaping a TimerTask kills the shared static Timer, + * which silently disables every alarm in every container in the process, so a missing callback + * has to be an ordinary null result instead. + */ + @Test + public void extractingAMissingCallbackReturnsNullInsteadOfThrowing() { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), "compiled XDK system modules are required"); + + ServiceContext context = RuntimeTestSupport.newContainer().createServiceContext("extract"); + WeakCallback ref = new WeakCallback(RuntimeTestSupport.entryFrame(context), null); + + assertNotNull(ref.extractCallback(), "the first extraction must hand back the callback"); + assertNull(ref.extractCallback(), + "extracting an already-extracted callback must not throw on the timer thread"); + assertTrue(context.getCallbackMap().isEmpty(), + "extraction must remove the entry from the registry"); + } + + /** + * The real cross-thread access pattern: the owning service registers callbacks on its own + * thread while previously scheduled alarms mature and extract them on the Java timer thread, + * with no monitor in common. Against the old plain HashMap a put that resizes the table racing + * a remove could lose an entry or corrupt the map outright. + */ + @Test + public void registryToleratesConcurrentServiceAndTimerThreadAccess() throws InterruptedException { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), "compiled XDK system modules are required"); + + ServiceContext context = RuntimeTestSupport.newContainer().createServiceContext("race"); + Frame frame = RuntimeTestSupport.entryFrame(context); + + var pending = new ArrayBlockingQueue(QUEUE_DEPTH); + var failures = new CopyOnWriteArrayList(); + var extracted = new AtomicInteger(); + + // stands in for the shared Java timer thread draining matured alarms + Thread timer = new Thread(() -> { + for (int i = 0; i < CALLBACK_COUNT; i++) { + try { + if (pending.take().extractCallback() != null) { + extracted.incrementAndGet(); + } + } catch (Throwable e) { + failures.add(e); + return; + } + } + }, "test-timer-thread"); + timer.setDaemon(true); + timer.start(); + + // this thread stands in for the owning service registering new alarms + for (int i = 0; i < CALLBACK_COUNT; i++) { + pending.put(new WeakCallback(frame, null)); + } + timer.join(JOIN_TIMEOUT_MILLIS); + + assertTrue(failures.isEmpty(), + () -> "the timer thread must never see a corrupt or lost registry entry, but got: " + + failures.getFirst()); + assertFalse(timer.isAlive(), + "the timer thread must not be stuck inside the callback registry"); + assertEquals(CALLBACK_COUNT, extracted.get(), + "every registered callback must be extractable exactly once"); + assertTrue(context.getCallbackMap().isEmpty(), + "a fully drained registry must be empty"); + } + + /** + * Keep-alive ownership is a lifecycle count that pins the container open while a native timer + * may still call back into it. Registration used to happen in the Alarm constructor, so when + * Timer.schedule(...) then failed, the recovery path called cancel() - whose unregister was + * gated on TimerTask.cancel(), which reports false for a task that was never scheduled. The + * count stayed up forever: the container could never go idle, so a "once and done" run could + * not terminate. + *

+ * This drives the real native "schedule" entry point with the shared timer forced into a state + * where scheduling fails. + */ + @Test + public void failedAlarmScheduleDoesNotPinTheContainerAlive() { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), "compiled XDK system modules are required"); + + NativeContainer container = RuntimeTestSupport.newContainer(); + ServiceContext context = container.createServiceContext("alarm"); + Frame frame = RuntimeTestSupport.entryFrame(context); + + GenericHandle hDelay = new GenericHandle( + container.getTemplate("temporal.Duration").getCanonicalClass()); + hDelay.setField(null, "picoseconds", + xInt128.INSTANCE.makeHandle(new LongLong(PICOS_PER_MILLI))); + + // keepAlive=True, so a successful schedule would pin the container until the alarm fires + ObjectHandle[] ahArg = new ObjectHandle[]{hDelay, null, xBoolean.TRUE}; + + assertTrue(container.isIdle(), "a container with no alarms must start out idle"); + + Timer timerLive = xLocalClock.TIMER; + Timer timerDead = new Timer("test-cancelled-timer", true); + timerDead.cancel(); + xLocalClock.TIMER = timerDead; + try { + int nResult = xLocalClock.INSTANCE.invokeNativeN(frame, + xLocalClock.INSTANCE.getStructure().findMethod("schedule", 3), + null, ahArg, Op.A_IGNORE); + + assertEquals(Op.R_EXCEPTION, nResult, + "a scheduler failure must be reported to natural code"); + assertNotNull(frame.m_hException, "the raised exception must be recorded on the frame"); + } finally { + xLocalClock.TIMER = timerLive; + } + + assertTrue(container.isIdle(), + "a failed alarm schedule must not leave the container pinned alive"); + } + + /** + * The NanoTimer variant of the same defect, with an extra failure of its own: scheduling ran + * under "catch (Throwable)" that cancelled the trigger and then fell through to return a + * perfectly ordinary "cancel" function. Natural code was told the alarm had been scheduled when + * it never would fire, and the keep-alive count stayed elevated on top of that. The scheduler + * failure has to reach natural code as an exception, with the registration unwound. + */ + @Test + public void failedNanosTimerScheduleIsReportedAndUnwound() { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), "compiled XDK system modules are required"); + + NativeContainer container = RuntimeTestSupport.newContainer(); + ServiceContext context = container.createServiceContext("nanos"); + Frame frame = RuntimeTestSupport.entryFrame(context); + + ObjectHandle hTimer = xNanosTimer.INSTANCE.ensureTimer(frame, null); + xNanosTimer.INSTANCE.invokeNativeN(frame, + xNanosTimer.INSTANCE.getStructure().findMethod("start", 0), + hTimer, Utils.OBJECTS_NONE, Op.A_IGNORE); + + GenericHandle hDelay = new GenericHandle( + container.getTemplate("temporal.Duration").getCanonicalClass()); + hDelay.setField(null, "picoseconds", + xInt128.INSTANCE.makeHandle(new LongLong(PICOS_PER_MILLI))); + + ObjectHandle[] ahArg = new ObjectHandle[]{hDelay, null, xBoolean.TRUE}; + + assertTrue(container.isIdle(), "a container with no alarms must start out idle"); + + Timer timerLive = xLocalClock.TIMER; + Timer timerDead = new Timer("test-cancelled-timer", true); + timerDead.cancel(); + xLocalClock.TIMER = timerDead; + try { + int nResult = xNanosTimer.INSTANCE.invokeNativeN(frame, + xNanosTimer.INSTANCE.getStructure().findMethod("schedule", 3), + hTimer, ahArg, Op.A_IGNORE); + + assertEquals(Op.R_EXCEPTION, nResult, + "a swallowed scheduler failure would hand natural code an alarm " + + "that can never fire"); + } finally { + xLocalClock.TIMER = timerLive; + } + + assertTrue(container.isIdle(), + "a failed alarm schedule must not leave the container pinned alive"); + } + + // ----- constants ----------------------------------------------------------------------------- + + private static final long PICOS_PER_MILLI = 1_000_000_000L; + + private static final int CALLBACK_COUNT = 50_000; + + private static final int QUEUE_DEPTH = 256; + + private static final long JOIN_TIMEOUT_MILLIS = 60_000L; +} diff --git a/javatools/src/test/java/org/xvm/runtime/RuntimeTestSupport.java b/javatools/src/test/java/org/xvm/runtime/RuntimeTestSupport.java new file mode 100644 index 0000000000..16d8310d61 --- /dev/null +++ b/javatools/src/test/java/org/xvm/runtime/RuntimeTestSupport.java @@ -0,0 +1,124 @@ +package org.xvm.runtime; + + +import java.io.File; + +import java.nio.file.Files; +import java.nio.file.Path; + +import java.util.List; +import java.util.Objects; + +import org.xvm.asm.Constants; +import org.xvm.asm.DirRepository; +import org.xvm.asm.LinkedRepository; +import org.xvm.asm.ModuleRepository; +import org.xvm.asm.Op; + +import org.xvm.asm.op.Return_0; + + +/** + * Shared fixture for tests that need a live runtime rather than a mock: it locates the compiled + * system modules in the gradle build outputs and boots a real primordial container over them. + *

+ * This lives in {@code org.xvm.runtime} because creating a native entry frame needs + * {@link ServiceContext#createServiceEntryFrame} and {@link ServiceContext.Message}, which are not + * visible outside the package. Tests in other packages call {@link #entryFrame} from here. + */ +public final class RuntimeTestSupport { + private RuntimeTestSupport() { + } + + /** + * @return a fresh primordial container over the compiled system modules + */ + public static NativeContainer newContainer() { + return new NativeContainer(new Runtime(), systemRepository()); + } + + /** + * Create a native entry frame for the specified service, of the shape a native method receives. + */ + public static Frame entryFrame(ServiceContext context) { + var message = new ServiceContext.Message(null) { + @Override + public boolean isAsync() { + return true; + } + + @Override + public int getCallDepth() { + return 0; + } + + @Override + public ObjectHandle getTimeoutHandle() { + return null; + } + + @Override + public long getTimeoutStamp() { + return 0L; + } + + @Override + Frame createFrame(ServiceContext ctx) { + return ctx.createServiceEntryFrame(this, 0, NATIVE_OPS); + } + }; + return context.createServiceEntryFrame(message, 0, NATIVE_OPS); + } + + /** + * @return true iff the compiled core modules these tests need are on disk + */ + public static boolean systemModulesAvailable() { + ModuleRepository repository = systemRepository(); + return repository != null + && repository.loadModule(Constants.ECSTASY_MODULE) != null + && repository.loadModule(Constants.TURTLE_MODULE) != null + && repository.loadModule(Constants.NATIVE_MODULE) != null; + } + + /** + * Test-only locator for the gradle build outputs that hold the compiled system modules. + */ + private static ModuleRepository systemRepository() { + var repositories = SYSTEM_MODULE_PATHS.stream() + .map(RuntimeTestSupport::repositoryFor) + .filter(Objects::nonNull) + .toList(); + return repositories.isEmpty() + ? null + : new LinkedRepository(repositories.toArray(ModuleRepository.NO_REPOS)); + } + + private static ModuleRepository repositoryFor(String path) { + File directory = checkoutFile(path); + return directory.isDirectory() ? new DirRepository(directory, true) : null; + } + + /** + * Resolve a checkout-relative path against the repository root, i.e. the nearest ancestor of + * the working directory that contains a {@code javatools} directory. + */ + private static File checkoutFile(String path) { + Path root = Path.of("").toAbsolutePath(); + while (root != null && !Files.isDirectory(root.resolve("javatools"))) { + root = root.getParent(); + } + return Objects.requireNonNull(root, "checkout root").resolve(path).toFile(); + } + + + // ----- constants ----------------------------------------------------------------------------- + + private static final List SYSTEM_MODULE_PATHS = List.of( + "lib_ecstasy/build/xtc/main/lib", + "javatools_bridge/build/xtc/main/lib", + "xdk/build/install/xdk/lib", + "xdk/build/install/xdk/javatools"); + + private static final Op[] NATIVE_OPS = new Op[]{Return_0.INSTANCE}; +} diff --git a/javatools/src/test/java/org/xvm/runtime/template/_native/web/HttpServerBindRollbackTest.java b/javatools/src/test/java/org/xvm/runtime/template/_native/web/HttpServerBindRollbackTest.java new file mode 100644 index 0000000000..cc23566cc9 --- /dev/null +++ b/javatools/src/test/java/org/xvm/runtime/template/_native/web/HttpServerBindRollbackTest.java @@ -0,0 +1,100 @@ +package org.xvm.runtime.template._native.web; + + +import com.sun.net.httpserver.HttpServer; + +import org.junit.jupiter.api.Test; + +import org.xvm.asm.Op; + +import org.xvm.runtime.Frame; +import org.xvm.runtime.NativeContainer; +import org.xvm.runtime.ObjectHandle; +import org.xvm.runtime.RuntimeTestSupport; +import org.xvm.runtime.ServiceContext; + +import org.xvm.runtime.template.numbers.xInt64; + +import org.xvm.runtime.template.text.xString; + +import org.xvm.runtime.template._native.web.xRTServer.HttpServerHandle; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import static org.junit.jupiter.api.Assumptions.assumeTrue; + + +/** + * Behavioral test for the server bind rollback path. Boots a real container, binds real HTTP and + * HTTPS servers on ephemeral ports, and then fails the very last step of the bind - after both + * servers are running and after the container keep-alive callback has been registered. + */ +public class HttpServerBindRollbackTest { + /** + * The bind sequence registers the container keep-alive callback before its final steps, so a + * failure in "createContext(...)" used to terminate the service context while leaving the + * callback count elevated, both started Java servers running, and the thread pool alive. The + * container could then never go idle, so a "once and done" run could not terminate. + *

+ * The fault is injected by clearing the router, which makes the real bind implementation reach + * "httpServer.createContext("/", null)" and raise NullPointerException at exactly the + * post-registration point the rollback exists for. Everything up to that point - socket + * binding, executor creation, starting both servers, registering the callback - is real. + */ + @Test + public void failedServerBindDoesNotPinTheContainerAlive() { + assumeTrue(RuntimeTestSupport.systemModulesAvailable(), + "compiled XDK system modules are required"); + + NativeContainer container = RuntimeTestSupport.newContainer(); + ServiceContext context = container.createServiceContext("HttpServer"); + Frame frame = RuntimeTestSupport.entryFrame(context); + + HttpServerHandle hServer = new HttpServerHandle( + xRTServer.INSTANCE.getCanonicalClass(), context); + + // drop the router so that the last step of the bind fails, after the servers have started + // and after the keep-alive registration + hServer.setRouter(null); + + // a null binding is required to match the cleared router; ports are 0 so the OS picks + // ephemeral ones and the test can never collide with anything else on the machine + ObjectHandle[] ahArg = new ObjectHandle[]{ + null, + xString.makeHandle("localhost"), + xInt64.makeHandle(0), + xInt64.makeHandle(0)}; + + assertTrue(container.isIdle(), "a container with no server must start out idle"); + + try { + int nResult = xRTServer.INSTANCE.invokeNativeN(frame, + xRTServer.INSTANCE.getStructure().findMethod("bindImpl", 4), + hServer, ahArg, Op.A_IGNORE); + + assertEquals(Op.R_EXCEPTION, nResult, "a failed bind must be reported to natural code"); + } finally { + // belt and braces: on the unfixed code the rollback does not happen, so the started + // servers would otherwise keep non-daemon threads alive for the rest of the JVM + stopQuietly(hServer.getHttpServer()); + stopQuietly(hServer.getHttpsServer()); + } + + assertTrue(container.isIdle(), + "a failed server bind must not leave the container pinned alive"); + assertNull(hServer.getHttpServer(), + "a failed server bind must leave the handle unconfigured, so a retry is possible"); + } + + private static void stopQuietly(HttpServer server) { + if (server != null) { + try { + server.stop(0); + } catch (RuntimeException _) { + // nothing useful to do in test cleanup + } + } + } +}