diff --git a/core/src/main/c/share/files.c b/core/src/main/c/share/files.c index 75f228fa..209c4490 100644 --- a/core/src/main/c/share/files.c +++ b/core/src/main/c/share/files.c @@ -236,6 +236,18 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_lock return flock((int) fd, LOCK_EX | LOCK_NB); } +JNIEXPORT jint JNICALL Java_io_questdb_client_cutlass_qwp_client_sf_cursor_SlotLock_release0 + (JNIEnv *e, jclass cl, jint fd) { + if (flock((int) fd, LOCK_UN) != 0) { + return -1; + } + /* Unlock success confirms that the slot is reusable. close() is one-shot: + * POSIX leaves descriptor state unspecified on EINTR, so retrying its + * numeric value could close an unrelated descriptor after reuse. */ + (void) close((int) fd); + return 0; +} + JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mkdir0 (JNIEnv *e, jclass cl, jlong lpszPath, jint mode) { return mkdir((const char *) (uintptr_t) lpszPath, (mode_t) mode); diff --git a/core/src/main/c/share/net.c b/core/src/main/c/share/net.c index 3b0162fc..9b58fd65 100644 --- a/core/src/main/c/share/net.c +++ b/core/src/main/c/share/net.c @@ -128,6 +128,11 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_send return com_questdb_network_Net_EOTHERDISCONNECT; } +JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_shutdown + (JNIEnv *e, jclass cl, jint fd) { + return shutdown((int) fd, SHUT_RDWR); +} + JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_recv (JNIEnv *e, jclass cl, jint fd, jlong ptr, jint len) { ssize_t n; diff --git a/core/src/main/c/share/net.h b/core/src/main/c/share/net.h index 27143639..d80617e2 100644 --- a/core/src/main/c/share/net.h +++ b/core/src/main/c/share/net.h @@ -80,6 +80,14 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_recv JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_send (JNIEnv *, jclass, jint, jlong, jint); +/* + * Class: io_questdb_client_network_Net + * Method: shutdown + * Signature: (I)I + */ +JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_shutdown + (JNIEnv *, jclass, jint); + /* * Class: io_questdb_client_network_Net * Method: getSndBuf diff --git a/core/src/main/c/windows/files.c b/core/src/main/c/windows/files.c index 119f6315..d1d3b413 100644 --- a/core/src/main/c/windows/files.c +++ b/core/src/main/c/windows/files.c @@ -331,6 +331,20 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_lock return 0; } +JNIEXPORT jint JNICALL Java_io_questdb_client_cutlass_qwp_client_sf_cursor_SlotLock_release0 + (JNIEnv *e, jclass cl, jint fd) { + OVERLAPPED ov; + memset(&ov, 0, sizeof(ov)); + if (!UnlockFileEx(FD_TO_HANDLE(fd), 0, MAXDWORD, MAXDWORD, &ov)) { + SaveLastError(); + return -1; + } + /* Unlock success confirms that the slot is reusable. Match POSIX by + * closing once without making handle cleanup part of that signal. */ + (void) CloseHandle(FD_TO_HANDLE(fd)); + return 0; +} + JNIEXPORT jint JNICALL Java_io_questdb_client_std_Files_mkdir0 (JNIEnv *e, jclass cl, jlong lpszPath, jint mode) { (void) mode; diff --git a/core/src/main/c/windows/net.c b/core/src/main/c/windows/net.c index fd290629..9cc98000 100644 --- a/core/src/main/c/windows/net.c +++ b/core/src/main/c/windows/net.c @@ -230,6 +230,15 @@ JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_configureNonBlocking return res; } +JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_shutdown + (JNIEnv *e, jclass cl, jint fd) { + const int result = shutdown((SOCKET) fd, SD_BOTH); + if (result == SOCKET_ERROR) { + SaveLastError(); + } + return result; +} + JNIEXPORT jint JNICALL Java_io_questdb_client_network_Net_recv (JNIEnv *e, jclass cl, jint fd, jlong addr, jint len) { const int n = recv((SOCKET) fd, (char *) addr, len, 0); diff --git a/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketClient.java b/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketClient.java index fd16dca7..94018fff 100644 --- a/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketClient.java +++ b/core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketClient.java @@ -248,6 +248,15 @@ public void close() { } } + /** + * Shuts down socket traffic without releasing the descriptor or freeing + * buffers that a concurrent I/O worker may still access. The owner must + * call {@link #close()} after joining the worker to complete cleanup. + */ + public void closeTraffic() { + socket.closeTraffic(); + } + /** * Connects to a WebSocket server. * diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java index d1744065..403748bf 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java @@ -189,11 +189,17 @@ public class QwpWebSocketSender implements Sender { // Null in production; set reflectively by tests. @TestOnly private volatile java.util.function.Supplier clientFactoryOverride; + // Test-only lifecycle witness. drainOnClose() invokes and clears it after + // confirming a real unacknowledged close target, immediately before waiting. + private volatile Runnable closeDrainWaitingHook; // close() drain timeout in millis. Default applied at construction. // 0 or -1 means "fast close" (skip the drain); otherwise close blocks // up to this many millis for ackedFsn to catch up to publishedFsn. private long closeFlushTimeoutMillis = 5_000L; private volatile boolean closed; + // Test-only lifecycle witness. close() invokes and clears it strictly after + // publishing closed=true and before starting any drain or teardown work. + private volatile Runnable closeStartedHook; private boolean connected; private SenderConnectionDispatcher connectionDispatcher; // Async-delivery sink for SenderConnectionEvent notifications. Default @@ -278,11 +284,23 @@ public class QwpWebSocketSender implements Sender { private boolean ownsCursorEngine; private long pendingBytes; // Set true by close() once the SF slot flock has been released (the normal - // teardown path). Stays false if close() bailed early with the I/O thread - // still running -- then cursorEngine.close() never ran and the flock is - // still held, so the owning pool MUST keep the slot reserved rather than - // hand the still-locked dir to the next borrow ("sf slot already in use"). - private boolean slotLockReleased; + // teardown path). Stays false if an I/O or manager worker did not stop and + // cursorEngine retained the flock, so the owning pool MUST keep the slot + // reserved rather than hand the still-locked dir to the next borrow + // ("sf slot already in use"). May flip to true LATER via the getter's + // re-probe of retainedEngine, once the deferred cleanup (manager-worker + // exit path or delegated I/O-thread close) releases the flock — pools + // re-probe retired slots to recover their capacity. volatile: written on + // the closing thread, read by pool threads. + private volatile boolean slotLockReleased; + // Optional owning-pool notification, relayed after the engine confirms the + // flock release and slotLockReleased is visible to pool re-probes. + private volatile Runnable slotLockReleaseListener; + // Engine whose close() could not complete during sender close() — its + // cleanup is pending on a worker/I/O-thread exit path. isSlotLockReleased() + // re-probes it so a late flock release becomes visible to the owning pool. + // Only ever set inside close(); null for a sender that closed cleanly. + private volatile CursorSendEngine retainedEngine; private int pendingRowCount; private SenderProgressDispatcher progressDispatcher; // Async-delivery sink for ack-watermark advances. Default no-op; a @@ -1038,13 +1056,26 @@ public QwpWebSocketSender charColumn(CharSequence columnName, char value) { * replaying frames get a 2.5s grace window plus a 0.5s stop window * — worst case ~3s when a drainer sits in a blocking native * connect (15s background deadline) and must be abandoned to exit - * on its own. + * on its own; + *
  • SF manager stop: normally immediate, bounded by 5s when its worker + * is stuck in a filesystem operation. On timeout the slot remains + * locked rather than being exposed to a stale worker.
    • * */ @Override public void close() { if (!closed) { closed = true; + Runnable hook = closeStartedHook; + closeStartedHook = null; + if (hook != null) { + try { + hook.run(); + } catch (Throwable t) { + // A test witness must never prevent production resource cleanup. + LOG.error("Error in close-started test hook: {}", String.valueOf(t)); + } + } boolean ioThreadStopped = true; // Captures the first error from the flush/drain path AND any // secondary errors from cleanup steps (added via addSuppressed). @@ -1187,17 +1218,34 @@ public void close() { // close actually runs, so the pool must not reuse the slot // meanwhile. A false return means the thread exited between // the failed close() and now — then closing here is safe. - if (ownsCursorEngine && cursorEngine != null && cursorSendLoop != null - && !cursorSendLoop.delegateEngineClose()) { - try { - cursorEngine.close(); - } catch (Throwable t) { - LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t)); - terminalError = captureCloseError(terminalError, t); + if (ownsCursorEngine && cursorEngine != null && cursorSendLoop != null) { + if (cursorSendLoop.delegateEngineClose()) { + // The I/O thread adopted the engine close and runs it on + // its exit path. Keep the engine visible for re-probing: + // isSlotLockReleased() flips true once that close + // completes, letting a pool recover the retired slot. + retainedEngine = cursorEngine; + } else { + CursorSendEngine engine = cursorEngine; + try { + engine.close(); + } catch (Throwable t) { + LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t)); + terminalError = captureCloseError(terminalError, t); + } + if (engine.isCloseCompleted()) { + cursorEngine = null; + ownsCursorEngine = false; + slotLockReleased = true; + } else { + // Engine cleanup is pending on its manager worker's + // exit path (or leaked, for a shared manager). Report + // the retained flock now; the getter re-probes the + // retained engine so a late release is not lost. + slotLockReleased = false; + retainedEngine = engine; + } } - cursorEngine = null; - ownsCursorEngine = false; - slotLockReleased = true; } rethrowTerminal(terminalError); return; @@ -1231,19 +1279,32 @@ public void close() { } if (ownsCursorEngine && cursorEngine != null) { + CursorSendEngine engine = cursorEngine; try { - cursorEngine.close(); + engine.close(); } catch (Throwable t) { LOG.error("Error closing owned CursorSendEngine: {}", String.valueOf(t)); terminalError = captureCloseError(terminalError, t); } - cursorEngine = null; - ownsCursorEngine = false; + if (engine.isCloseCompleted()) { + cursorEngine = null; + ownsCursorEngine = false; + slotLockReleased = true; + } else { + // The manager worker did not quiesce. Preserve ownership + // and report the retained flock so pools retire this slot. + // Repeated Sender.close() calls remain no-ops by contract. + // Engine cleanup was handed to a safe manager-worker path: + // owned-manager exit or shared-manager ring-pass completion. + // The getter re-probes the retained engine so the pool can + // reclaim the slot once cleanup actually completes. + slotLockReleased = false; + retainedEngine = engine; + } + } else { + // This sender owns no cursor engine holding an SF flock. + slotLockReleased = true; } - // Past the ioThreadStopped guard => cursorEngine.close() ran and - // released the SF flock in its finally (or this sender owned no - // engine holding one). Signal the pool it may reuse the slot. - slotLockReleased = true; // Shutdown order: dispatcher last, after the I/O loop has stopped // producing into it. close() drains pending entries with a short @@ -1287,13 +1348,51 @@ public void close() { } /** - * True once {@link #close()} has released the store-and-forward slot - * flock. False means close() leaked the still-running I/O thread (and its - * resources), so the flock is still held; the owning pool must keep the - * slot index reserved instead of reusing the still-locked slot dir. + * True once the store-and-forward slot flock has been released. False + * means an I/O or manager worker did not stop and close() retained the + * lock and worker-reachable resources; the owning pool must keep the slot + * index reserved instead of reusing the still-locked dir. + *

    + * Not a one-shot snapshot: when close() left engine cleanup pending on a + * manager-worker quiescence or I/O-thread exit path, this re-probes the + * retained engine and latches true the moment that cleanup completes — pools re-probe retired + * slots through this getter to recover their capacity. Monotonic: + * false→true only, never back. Cheap (volatile reads on every common + * path) so pools may call it under their capacity lock; only the rare + * orphaned-retry state below does more. + *

    + * The probe is also the recovery surface for a retained engine whose + * flock-release retry fell off the shared driver because the driver + * thread could not start (e.g. OOM at thread creation): close() is + * one-shot, so without the re-arm below that slot's capacity would stay + * lost until process exit. */ public boolean isSlotLockReleased() { - return slotLockReleased; + if (slotLockReleased) { + return true; + } + CursorSendEngine engine = retainedEngine; + if (engine != null) { + if (engine.isCloseCompleted()) { + // Benign latch race: concurrent callers may both observe the + // completed cleanup and both write true. + slotLockReleased = true; + return true; + } + engine.ensureFlockReleaseRetryScheduled(); + } + return false; + } + + /** + * Registers a callback for confirmed SF slot-lock release. Pools use this + * to wake borrowers without waiting for a timeout or housekeeper tick. + */ + public void setSlotLockReleaseListener(Runnable listener) { + slotLockReleaseListener = listener; + if (listener != null && slotLockReleased) { + listener.run(); + } } @Override @@ -2135,6 +2234,26 @@ public void setClientFactoryOverride(java.util.function.Supplier= target) { return; } + Runnable hook = closeDrainWaitingHook; + closeDrainWaitingHook = null; + if (hook != null) { + try { + hook.run(); + } catch (Throwable t) { + // A test witness must never prevent production resource cleanup. + LOG.error("Error in close-drain-waiting test hook: {}", String.valueOf(t)); + } + } long deadlineNanos = System.nanoTime() + closeFlushTimeoutMillis * 1_000_000L; while (cursorEngine.ackedFsn() < target) { // Stop on a latched terminal (acks will never reach target); @@ -3545,6 +3682,14 @@ private void flushPendingRowsSplit(ObjList keys, boolean deferComm resetTableBuffersAfterFlush(keys); } + private void onSlotLockReleased() { + slotLockReleased = true; + Runnable listener = slotLockReleaseListener; + if (listener != null) { + listener.run(); + } + } + private void resetTableBuffersAfterFlush(ObjList keys) { for (int i = 0, n = keys.size(); i < n; i++) { CharSequence tableName = keys.getQuick(i); diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java index 64ca75d0..01910fcb 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorSendEngine.java @@ -28,8 +28,14 @@ import io.questdb.client.std.Files; import io.questdb.client.std.ObjList; import io.questdb.client.std.QuietCloseable; +import org.jetbrains.annotations.TestOnly; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.LockSupport; +import java.util.function.LongConsumer; /** * Facade that bundles a {@link SegmentRing} with a {@link SegmentManager} and @@ -63,6 +69,18 @@ public final class CursorSendEngine implements QuietCloseable { public static final long DEFAULT_APPEND_DEADLINE_NANOS = 30_000_000_000L; private static final org.slf4j.Logger LOG = org.slf4j.LoggerFactory.getLogger(CursorSendEngine.class); + private static final ThreadFactory DEFAULT_FLOCK_RELEASE_RETRY_THREAD_FACTORY = + runnable -> new Thread(runnable, "qdb-sf-flock-release-retry"); + private static final long FLOCK_RELEASE_RETRY_BASE_PARK_NANOS = 100_000_000L; // 100 ms + private static final Object FLOCK_RELEASE_RETRY_LOCK = new Object(); + private static final long FLOCK_RELEASE_RETRY_MAX_PARK_NANOS = 5_000_000_000L; // 5 s + private static final ArrayDeque FLOCK_RELEASE_RETRY_QUEUE = new ArrayDeque<>(); + private static volatile Runnable afterFlockReleaseRetryFailureHook; + private static volatile Runnable beforeDeferredCloseCreationHook; + private static volatile LongConsumer flockReleaseRetryParkOverride; + private static Thread flockReleaseRetryThread; + private static volatile ThreadFactory flockReleaseRetryThreadFactory = + DEFAULT_FLOCK_RELEASE_RETRY_THREAD_FACTORY; private final long appendDeadlineNanos; // Number of times appendBlocking observed BACKPRESSURE_NO_SPARE on its first // ring.appendOrFsn attempt. One increment per blocking-call that had to wait @@ -70,6 +88,10 @@ public final class CursorSendEngine implements QuietCloseable { // writer; volatile because the user may sample it from any thread. private final java.util.concurrent.atomic.AtomicLong backpressureStallCount = new java.util.concurrent.atomic.AtomicLong(); + // Constructed before an owned manager acquires its native path scratch, so + // callback allocation failure cannot orphan manager resources. A timed-out + // close can then hand it to either manager path without allocating. + private final Runnable deferredClose; private final SegmentManager manager; // We own the manager iff the user constructed us with no manager — in that // case close() also stops the manager. When the manager is shared across @@ -114,6 +136,51 @@ public final class CursorSendEngine implements QuietCloseable { // thread, JVM shutdown hooks, test cleanup). volatile + synchronized // close() makes the check-and-set atomic and gives readers a fence. private volatile boolean closed; + // True once close() has run its full cleanup sequence INCLUDING a + // CONFIRMED slot-flock release — finishClose() publishes this strictly + // after SlotLock.release() reports success, never before. Pool threads + // treat the flip as "the slot dir is reusable" and free the slot index + // the moment they observe it (QwpWebSocketSender.isSlotLockReleased -> + // SenderPool.reprobeRetiredSlots), so publishing before the release + // would let a replacement engine's SlotLock.acquire collide with the + // still-open fd. Stays false when a close attempt could not confirm + // manager-worker quiescence (or the flock release itself failed) and had + // to leak the ring/watermark/slot lock — in that case a later close() + // call retries the cleanup (the worker may have exited by then). + // volatile: latched by finishClose(), but read lock-free by + // isCloseCompleted() from pool threads re-probing a retired slot (see the + // getter for why it must not synchronize). + private volatile boolean closeCompleted; + // Invoked after closeCompleted publishes a confirmed flock release. Used + // by an owning sender pool to wake capacity-starved borrowers immediately. + private volatile Runnable slotLockReleaseListener; + // Test-only hook run by finishClose() between the terminal cleanup and + // the flock release. Lets a test park the releasing thread inside the + // cleanup/release window and assert that closeCompleted stays false — + // i.e. that completion is never observable while the flock is still + // held. volatile: finishClose may run on the manager worker's exit + // thread while the hook is installed from a test thread. + private volatile Runnable beforeFlockReleaseHook; + // Ensures this engine has at most one entry in the shared flock-release + // retry driver. The error path only: ordinary closes never enqueue work. + private final AtomicBoolean flockReleaseRetryStarted = new AtomicBoolean(); + // Published before deferredClose is registered. The manager lock provides + // the callback handoff fence; volatile also covers a direct test/retry read. + private volatile boolean fullyDrainedForDeferredClose; + // Exactly-once claim on the terminal cleanup (finishClose). Contended by + // close() and a worker-exit handoff (completeDeferredClose); whoever wins + // the CAS runs the cleanup, the loser never touches ring/watermark/flock. + // Deliberately NOT the engine monitor: a retried close() holds the + // monitor while joining the manager worker, and the worker cannot die + // until its exit-path cleanup finishes — monitor-based exclusion would + // stall that close() for its full join budget (test-visible livelock). + // With the CAS the worker's cleanup never blocks, so the join returns as + // soon as the pass ends. + private final AtomicBoolean terminalCleanupClaimed = new AtomicBoolean(); + // Published only after ring/watermark/unlink cleanup is finished. A close + // that loses terminalCleanupClaimed may retry the flock only after this + // becomes true, otherwise it could expose the slot while cleanup is live. + private volatile boolean terminalResourcesCleaned; // Producer-thread-only: timestamp of the last "we're backpressured" log // line, used to throttle. Plain long is fine. private long lastBackpressureLogNs; @@ -137,9 +204,7 @@ public CursorSendEngine(String sfDir, long segmentSizeBytes) { */ public CursorSendEngine(String sfDir, long segmentSizeBytes, long maxTotalBytes, long appendDeadlineNanos) { - this(sfDir, segmentSizeBytes, - new SegmentManager(segmentSizeBytes, SegmentManager.DEFAULT_POLL_NANOS, maxTotalBytes), - true, appendDeadlineNanos); + this(sfDir, segmentSizeBytes, null, true, maxTotalBytes, appendDeadlineNanos); } /** @@ -153,6 +218,22 @@ public CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager mana private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager manager, boolean ownsManager, long appendDeadlineNanos) { + this(sfDir, segmentSizeBytes, manager, ownsManager, + SegmentManager.UNLIMITED_TOTAL_BYTES, appendDeadlineNanos); + } + + private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager manager, + boolean ownsManager, long maxTotalBytes, long appendDeadlineNanos) { + // Allocate the bound callback before constructing an owned manager. + // Field initializers have completed, but no engine-owned native/disk + // resource exists yet. If callback allocation throws, construction + // stops without a manager whose native path scratch could be orphaned. + this.deferredClose = createDeferredClose(); + if (ownsManager && manager == null) { + manager = new SegmentManager( + segmentSizeBytes, SegmentManager.DEFAULT_POLL_NANOS, maxTotalBytes); + } + // sfDir == null → memory-only mode (non-SF async ingest). Same // cursor architecture, no disk involvement; segments // live in malloc'd native memory. @@ -172,11 +253,9 @@ private CursorSendEngine(String sfDir, long segmentSizeBytes, SegmentManager man // separate mkdir step needed here. acquiredLock = SlotLock.acquire(sfDir); } catch (Throwable t) { - // The delegating constructors evaluate `new SegmentManager(...)` - // BEFORE this body runs, so on a pre-try throw (e.g. slot lock - // collision) an owned manager is already alive and would leak - // its native path-scratch sink -- 256 bytes per failed - // construction attempt. Close it before propagating. + // Callback creation and owned-manager construction have already + // completed. A slot-lock failure must close the owned manager's + // native path scratch before propagating. if (ownsManager) { try { manager.close(); @@ -480,7 +559,7 @@ public long appendOrFsn(long payloadAddr, int payloadLen, long spinDeadlineNanos @Override public synchronized void close() { - if (closed) return; + if (closed && closeCompleted) return; closed = true; // Capture drain state BEFORE closing the ring — once the ring is // closed, its accessors aren't safe to read. The active segment is @@ -491,35 +570,201 @@ public synchronized void close() { // against potentially-fresh server state — duplicate writes when // the server has no dedup state for those messageSequences. // Memory mode has no files to unlink. - // The whole close sequence runs under try/finally so the slot lock - // is ALWAYS released, even if manager/ring close or unlink throws — - // otherwise a kernel-held flock outlives the engine and the next - // sender for the same slot collides on a lock the dead engine - // never released. + // + // Cleanup is gated on worker quiescence: releasing the ring, + // watermark, segment files or the slot lock while the manager worker + // is still mid service pass would let a replacement engine acquire + // the slot and have its files unlinked by the stale worker — + // store-and-forward data loss after restart. When the bounded worker + // join times out on an owned manager, cleanup OWNERSHIP TRANSFERS to + // the worker's exit path (deferUntilWorkerExit): the worker is + // provably the last thread able to touch the slot directory, so it + // releases everything the moment its in-flight pass finishes instead + // of leaking the slot until process exit. + // + // "Fully drained" includes BOTH the obvious case (every published + // FSN has been acked) AND the never-published case (publishedFsn + // < 0). The latter matters because a drainer adopting an empty + // orphan slot — segments filtered as empty by recovery, engine + // recreates a fresh sf-initial.sfa — would otherwise leave that + // fresh empty file behind, the next scanner finds it, adopts the + // slot again, and the cycle repeats forever (M6). + // Own try/catch so sabotaged/broken ring state cannot skip the + // quiescence barrier below or the slot-lock release in finishClose. + boolean drained = false; try { - // "Fully drained" includes BOTH the obvious case (every published - // FSN has been acked) AND the never-published case (publishedFsn - // < 0). The latter matters because a drainer adopting an empty - // orphan slot — segments filtered as empty by recovery, engine - // recreates a fresh sf-initial.sfa — would otherwise leave that - // fresh empty file behind, the next scanner finds it, adopts the - // slot again, and the cycle repeats forever (M6). - boolean fullyDrained = sfDir != null + drained = sfDir != null && (ring.publishedFsn() < 0 || ring.ackedFsn() >= ring.publishedFsn()); - // Each cleanup step in its own try/catch so a single failure - // doesn't strand later cleanups — mirrors the constructor's - // catch block. Without this, a throw from manager.deregister - // or manager.close() would leave the ring mmap'd and any - // residual .sfa files on disk, where the next sender can - // adopt them and replay already-acked data. + } catch (Throwable ignored) { + } + final boolean fullyDrained = drained; + fullyDrainedForDeferredClose = fullyDrained; + // Each cleanup step in its own try/catch so a single failure + // doesn't strand later cleanups — mirrors the constructor's + // catch block. Without this, a throw from manager.deregister + // or manager.close() would leave the ring mmap'd and any + // residual .sfa files on disk, where the next sender can + // adopt them and replay already-acked data. + try { + manager.deregister(ring); + } catch (Throwable ignored) { + } + // Quiescence barrier. deregister alone only removes the entry + // from the registry — the worker may still be mid service pass + // for this ring (creating a spare file, trimming, unlinking). + boolean workerQuiescent = false; + if (ownsManager) { + // Stopping and reaping a private manager is a stronger barrier + // than waiting for this ring alone. Do it directly so a stuck + // worker consumes at most one workerJoinTimeoutMillis budget, + // rather than one here and a second one in manager.close(). try { - manager.deregister(ring); + manager.close(); } catch (Throwable ignored) { } - if (ownsManager) { + try { + workerQuiescent = manager.isWorkerReaped(); + } catch (Throwable ignored) { + } + } else { + // A shared manager must keep serving its other rings, so wait + // only for the deregistered ring's current pass to finish. + try { + workerQuiescent = manager.awaitRingQuiescence(ring); + } catch (Throwable ignored) { + } + } + if (!workerQuiescent && ownsManager) { + // Ownership handoff: manager.close() already stopped the worker + // loop (running=false), so the worker exits as soon as its + // in-flight pass finishes — it is merely slow, or wedged in a + // syscall. Either way it is the last thread able to touch the + // slot, so hand it the terminal cleanup instead of leaking the + // slot until process exit. completeDeferredClose and a retried + // close() race through the terminalCleanupClaimed CAS, so the + // cleanup runs exactly once and the worker never blocks on the + // engine monitor a retried close() holds while joining it. + boolean handedOff = false; + boolean registrationFailed = false; + try { + handedOff = manager.deferOwnedEngineCloseUntilWorkerExit(deferredClose); + } catch (Throwable ignored) { + // Unexpected monitor/VM failure carries no worker-liveness + // information. Ordinary handoff cannot allocate: both the + // callback and the manager's single slot were preallocated. + registrationFailed = true; + } + if (handedOff) { + LOG.error("SF manager worker did not quiesce during engine close; ring, watermark " + + "and slot-lock release are handed to the worker's exit path and run the " + + "moment its in-flight service pass finishes. The slot stays locked (and " + + "isCloseCompleted() false) until then, so no replacement engine can race " + + "the stale worker on slot {}", sfDir == null ? "" : sfDir); + return; + } + if (registrationFailed) { + // The handoff never registered and the worker was never + // observed — it must be presumed live and mid service pass. + // Retain every worker-reachable resource (ring, watermark, + // segment files, slot flock) and leave terminalCleanupClaimed + // unclaimed and closeCompleted false, exactly like the + // shared-manager leak branch: manager.close() above already + // stopped the worker loop, so a retried close() converges via + // isWorkerReaped() once the in-flight pass ends. The kernel + // releases the slot flock on process exit regardless. + LOG.error("SF worker-exit handoff registration failed during engine close; " + + "leaking the ring, watermark and slot lock so a possibly-live " + + "worker cannot corrupt a future engine on slot {}. close() may be " + + "invoked again to retry cleanup once the worker has exited.", + sfDir == null ? "" : sfDir); + return; + } + // Handoff rejected: the worker loop exited between the failed + // bounded join and the registration attempt (both sides share the + // manager's lock, so this observation is exact). A worker past + // its loop can never touch slot paths again — inline cleanup is + // as safe as a reaped worker. + workerQuiescent = true; + } + if (!workerQuiescent) { + // A shared manager keeps serving sibling rings, so it cannot use + // the whole-worker exit handoff above. Transfer cleanup to this + // ring's current pass instead. Registration and pass completion + // share the manager lock: true means the worker owns cleanup; + // false means the pass already finished and inline cleanup is safe. + boolean handedOff = false; + boolean registrationFailed = false; + try { + handedOff = manager.deferUntilRingQuiescent(ring, deferredClose); + } catch (Throwable ignored) { + registrationFailed = true; + } + if (handedOff) { + LOG.error("SF manager worker did not quiesce during engine close; ring, watermark " + + "and slot-lock release are handed to the current ring pass and run the " + + "moment that pass finishes. The slot stays locked (and " + + "isCloseCompleted() false) until then, so no replacement engine can " + + "race the stale worker on slot {}", + sfDir == null ? "" : sfDir); + return; + } + if (registrationFailed) { + LOG.error("SF ring-pass cleanup handoff registration failed during engine close; " + + "leaking the ring, watermark and slot lock so a possibly-live worker " + + "cannot corrupt a future engine on slot {}. The kernel releases the " + + "flock on process exit.", sfDir == null ? "" : sfDir); + return; + } + workerQuiescent = true; + } + if (!terminalCleanupClaimed.compareAndSet(false, true)) { + // A worker-exit handoff or earlier close owns the one-time + // ring/watermark cleanup. Once that work is published complete, + // this close may still retry an unconfirmed flock release. + retryFlockReleaseIfReady(); + return; + } + finishClose(fullyDrained); + } + + /** + * Terminal cleanup: closes the ring and watermark, unlinks drained + * segment files, releases the slot flock, and — only once the release + * is confirmed — latches {@link #closeCompleted}. Publish order + * is load-bearing: pools free the slot index the instant they observe + * {@code closeCompleted} (via {@code isSlotLockReleased()}), so it must + * never be visible while the flock is still held, or a replacement + * engine races the release and fails acquisition on a live slot. + * The caller must hold the engine monitor and + * must have established that the manager worker can no longer touch the + * slot directory (reaped, provably exited, or running this on its own + * exit path) AND have won the {@link #terminalCleanupClaimed} CAS — the + * claim token, not the engine monitor, is the exclusion between a + * worker-exit handoff and a retried close(), so ring/watermark/flock are + * never double-released and the worker's cleanup can never deadlock + * against a close() that holds the monitor while joining the worker. + */ + private void finishClose(boolean fullyDrained) { + try { + // On a fully-drained close, persist the final acked FSN through + // the still-mapped watermark BEFORE closing the ring/watermark + // and BEFORE unlinking any segment file. The manager persists + // the watermark only on its own tick, so it may lag the final + // ack. If the unlink below then fails (or the process dies + // mid-unlink), residual acknowledged .sfa files without an + // up-to-date watermark would seed the successor's recovery at + // lowestBase - 1 and replay already-acknowledged rows -- + // duplicates on a non-DEDUP table. The write is a single mmap + // store, so it succeeds even when the unlink is about to fail + // (e.g. the slot dir turned read-only). Quiescence is already + // established here, so no manager tick can race this write. + if (fullyDrained && watermark != null) { try { - manager.close(); + long finalAckedFsn = ring.ackedFsn(); + if (finalAckedFsn >= 0) { + watermark.write(finalAckedFsn); + } } catch (Throwable ignored) { } } @@ -542,24 +787,326 @@ public synchronized void close() { } } if (fullyDrained) { + boolean segmentsRemoved = false; try { - unlinkAllSegmentFiles(sfDir); + segmentsRemoved = unlinkAllSegmentFiles(sfDir); } catch (Throwable ignored) { } + // Remove the watermark ONLY once every segment file is + // confirmed gone. The watermark is what keeps residual + // acknowledged segments inert to a successor's recovery; + // removing it while any .sfa file survives would republish + // those already-acknowledged rows. + if (segmentsRemoved) { + try { + AckWatermark.removeOrphan(sfDir); + } catch (Throwable ignored) { + } + } else { + LOG.warn("close-time segment cleanup incomplete on slot {}; retaining the ack " + + "watermark so residual acknowledged segments stay covered -- the next " + + "engine on this slot recovers them as fully acked and retries the " + + "unlink on its own close", sfDir); + } + } + } finally { + // Reaching finishClose at all requires established quiescence, so + // releasing the flock is safe even if a step above threw. Leaking + // it would strand the slot until process exit for no reason. + // + // ORDER MATTERS: explicitly release the flock, verify it, and + // only then publish closeCompleted. Pools read isCloseCompleted() + // as "the slot dir is reusable" and free the slot index the moment + // it flips. SlotLock closes the fd once after confirmed unlock, + // but that close result cannot safely control publication because + // POSIX may consume the fd even when close reports failure. + Runnable hook = beforeFlockReleaseHook; + if (hook != null) { try { - AckWatermark.removeOrphan(sfDir); + hook.run(); } catch (Throwable ignored) { + // test-only; must never block the release } } - } finally { - if (slotLock != null) { + terminalResourcesCleaned = true; + retryFlockReleaseIfReady(); + } + } + + /** + * Installs a hook invoked after each failed shared-driver flock release. + * Test-only: makes persistent retry progress observable without sleeps. + */ + @TestOnly + public static void setAfterFlockReleaseRetryFailureHook(Runnable hook) { + afterFlockReleaseRetryFailureHook = hook; + } + + /** + * Installs a constructor fault hook immediately before the bound deferred + * close callback is created. Test-only: proves callback allocation failure + * occurs before an owned manager acquires native resources. + */ + @TestOnly + public static void setBeforeDeferredCloseCreationHook(Runnable hook) { + beforeDeferredCloseCreationHook = hook; + } + + /** + * Replaces the shared retry driver's inter-round park with a callback + * receiving the park duration the driver would have used. Test-only: + * makes the retry cadence inspectable and rounds coordinatable without + * wall-clock waits. + */ + @TestOnly + public static void setFlockReleaseRetryParkOverride(LongConsumer override) { + flockReleaseRetryParkOverride = override; + } + + /** + * Overrides creation of the single shared flock-release retry thread. + * Test-only: makes thread creation/start failure and persistent retry + * scalability deterministic without relying on scheduler timing. + */ + @TestOnly + public static void setFlockReleaseRetryThreadFactory(ThreadFactory factory) { + synchronized (FLOCK_RELEASE_RETRY_LOCK) { + if (flockReleaseRetryThread != null || !FLOCK_RELEASE_RETRY_QUEUE.isEmpty()) { + throw new IllegalStateException("flock-release retry driver is active"); + } + flockReleaseRetryThreadFactory = factory == null + ? DEFAULT_FLOCK_RELEASE_RETRY_THREAD_FACTORY + : factory; + } + } + + /** + * Installs a hook that {@link #finishClose} runs between the terminal + * cleanup and the slot-flock release. Test-only: makes the otherwise + * microsecond-wide cleanup/release window deterministic so tests can + * assert {@link #isCloseCompleted()} stays false until the release is + * confirmed. + */ + @TestOnly + public void setBeforeFlockReleaseHook(Runnable hook) { + this.beforeFlockReleaseHook = hook; + } + + /** + * Runs on a safe manager-worker handoff path when {@link #close()} moved + * cleanup ownership to either a shared manager's ring-pass completion or + * an owned manager's worker exit. + * Deliberately does NOT take the engine monitor: a retried close() can + * hold it while joining this very worker, and the thread cannot die until + * this method returns — the {@link #terminalCleanupClaimed} CAS provides + * the exactly-once exclusion instead, so the worker always exits promptly + * and the racing close() converges via {@code isWorkerReaped()}. + */ + private void completeDeferredClose() { + if (!terminalCleanupClaimed.compareAndSet(false, true)) { + // A retried close() (or an earlier duplicate handoff) already ran + // the terminal cleanup. + return; + } + finishClose(fullyDrainedForDeferredClose); + LOG.info("deferred SF engine resource cleanup completed after manager-worker quiescence; " + + "slot release confirmed: {} [slot={}]", + closeCompleted, sfDir == null ? "" : sfDir); + } + + private Runnable createDeferredClose() { + Runnable hook = beforeDeferredCloseCreationHook; + if (hook != null) { + hook.run(); + } + return this::completeDeferredClose; + } + + private boolean retryFlockReleaseIfReady() { + if (closeCompleted || !terminalResourcesCleaned) { + return closeCompleted; + } + boolean released; + if (slotLock != null) { + try { + released = slotLock.release(); + } catch (Throwable ignored) { + released = false; + } + } else { + released = true; + } + if (released) { + closeCompleted = true; + Runnable listener = slotLockReleaseListener; + if (listener != null) { try { - slotLock.close(); + listener.run(); } catch (Throwable ignored) { - // best-effort; flock is also released by kernel on process exit + // A notification failure must not invalidate a confirmed release. + } + } + return true; + } + startFlockReleaseRetry(); + return false; + } + + private static void runFlockReleaseRetryDriver() { + // Capped exponential backoff: a persistent unlock failure must not + // burn a fixed 10 rounds of native release syscalls per second + // forever, but a transient failure must still recover promptly. The + // ramp doubles per fully-failed round from 100 ms up to 5 s. + long parkNanos = FLOCK_RELEASE_RETRY_BASE_PARK_NANOS; + while (true) { + final int roundSize; + synchronized (FLOCK_RELEASE_RETRY_LOCK) { + roundSize = FLOCK_RELEASE_RETRY_QUEUE.size(); + if (roundSize == 0) { + flockReleaseRetryThread = null; + return; + } + } + boolean hasFailures = false; + boolean hasSuccesses = false; + for (int i = 0; i < roundSize; i++) { + final CursorSendEngine engine; + synchronized (FLOCK_RELEASE_RETRY_LOCK) { + engine = FLOCK_RELEASE_RETRY_QUEUE.pollFirst(); + } + if (engine.retryFlockReleaseIfReady()) { + engine.flockReleaseRetryStarted.set(false); + hasSuccesses = true; + } else { + synchronized (FLOCK_RELEASE_RETRY_LOCK) { + FLOCK_RELEASE_RETRY_QUEUE.addLast(engine); + } + hasFailures = true; + Runnable hook = afterFlockReleaseRetryFailureHook; + if (hook != null) { + hook.run(); + } + } + } + if (hasSuccesses) { + // Progress: the failure condition is clearing, so retry the + // remaining engines on the base cadence again. + parkNanos = FLOCK_RELEASE_RETRY_BASE_PARK_NANOS; + } + if (hasFailures) { + // Interruption must not abandon a retained flock, but clear + // the flag so subsequent parks still throttle retries. + Thread.interrupted(); + LongConsumer parkOverride = flockReleaseRetryParkOverride; + if (parkOverride != null) { + parkOverride.accept(parkNanos); + } else { + LockSupport.parkNanos(parkNanos); + } + parkNanos = Math.min(parkNanos * 2, FLOCK_RELEASE_RETRY_MAX_PARK_NANOS); + } + } + } + + private void startFlockReleaseRetry() { + if (!flockReleaseRetryStarted.compareAndSet(false, true)) { + return; + } + Throwable startFailure = null; + synchronized (FLOCK_RELEASE_RETRY_LOCK) { + FLOCK_RELEASE_RETRY_QUEUE.addLast(this); + if (flockReleaseRetryThread != null) { + // The driver may be parked on a ramped backoff; wake it so a + // freshly failed release gets its first driver retry promptly + // instead of inheriting older engines' full backoff. + LockSupport.unpark(flockReleaseRetryThread); + } else { + try { + Thread retryThread = flockReleaseRetryThreadFactory.newThread( + CursorSendEngine::runFlockReleaseRetryDriver); + if (retryThread == null) { + throw new IllegalStateException("retry thread factory returned null"); + } + retryThread.setDaemon(true); + flockReleaseRetryThread = retryThread; + retryThread.start(); + } catch (Throwable t) { + startFailure = t; + flockReleaseRetryThread = null; + CursorSendEngine queued; + while ((queued = FLOCK_RELEASE_RETRY_QUEUE.pollFirst()) != null) { + queued.flockReleaseRetryStarted.set(false); + } } } } + if (startFailure == null) { + LOG.error("SF slot flock release failed during engine close; keeping " + + "closeCompleted=false and retrying on the shared driver so " + + "retired capacity recovers after the transient failure [slot={}]", + sfDir == null ? "" : sfDir); + } else { + // A later explicit close() or a pool's retired-slot probe + // (ensureFlockReleaseRetryScheduled) can still retry without + // repeating the one-time ring/watermark cleanup. The failed queue + // is cleared so the driver does not retain engines it cannot + // service. + LOG.error("Could not start SF flock-release retry driver; a retried close() " + + "or pool re-probe re-arms the retry [slot={}, error={}]", + sfDir == null ? "" : sfDir, String.valueOf(startFailure)); + } + } + + /** + * Whether {@link #close()} completed all cleanup, including a + * confirmed release of the SF slot lock — the flip is published + * strictly after explicit unlock succeeds, so observing {@code true} + * guarantees the slot dir is acquirable by a replacement engine. A false + * value after close means manager-worker quiescence could not be + * confirmed (or the flock release itself failed) and the + * worker-reachable resources were retained deliberately — either handed to the worker's exit path (owned manager), + * which flips this to true the moment the worker's in-flight pass + * finishes, or leaked until a retried close() (shared manager). Owners + * must not reuse the slot while this is false; pools may re-probe it to + * recover a retired slot's capacity once it flips. + *

    + * Deliberately unsynchronized ({@code closeCompleted} is volatile): pools + * probe this under their own capacity lock, and the deferred cleanup can + * hold the engine monitor through munmap/unlink syscalls — a synchronized + * getter would stall the pool's hot borrow path behind them. + */ + public boolean isCloseCompleted() { + return closeCompleted; + } + + /** + * Registers a callback for confirmed SF slot-lock release. If release + * already completed, invokes the callback before returning. + */ + public void setSlotLockReleaseListener(Runnable listener) { + slotLockReleaseListener = listener; + if (listener != null && closeCompleted) { + listener.run(); + } + } + + /** + * Re-arms the shared flock-release retry for an engine whose terminal + * cleanup finished but whose confirmed flock release is still pending + * and no longer scheduled — the retry driver thread failed to start when + * the release first failed (e.g. OOM at thread creation). + * {@code Sender.close()} is one-shot by contract, so pool probes + * ({@code QwpWebSocketSender.isSlotLockReleased()}) call this to keep a + * retired slot's capacity recoverable instead of lost until process + * exit. Cheap unless the engine is in that orphan state (volatile reads, + * then one failed CAS when the retry is already scheduled), so probes + * may call it under their capacity lock. + */ + public void ensureFlockReleaseRetryScheduled() { + if (closeCompleted || !terminalResourcesCleaned) { + return; + } + startFlockReleaseRetry(); } /** @@ -659,37 +1206,91 @@ public long recoveredOrphanTipFsn() { return recoveredOrphanTipFsn; } + /** + * Ascending removal rank of a segment file name for + * {@link #unlinkAllSegmentFiles(String)}. {@code sf-initial.sfa} is + * always the fresh-start segment at baseSeq 0, so it ranks first. + * Spare files carry a monotonic generation ({@code sf-.sfa}) + * assigned in creation == rotation == baseSeq order, so the parsed + * generation ranks them. Anything else with the extension is not a + * live segment and ranks last. + */ + private static long segmentCleanupRank(String name) { + if ("sf-initial.sfa".equals(name)) { + return Long.MIN_VALUE; + } + if (name.length() == 23 && name.startsWith("sf-")) { + try { + return Long.parseUnsignedLong(name.substring(3, 19), 16); + } catch (NumberFormatException ignored) { + // fall through to the unrecognized-name rank + } + } + return Long.MAX_VALUE; + } + /** * Unlinks every {@code .sfa} file under {@code dir}. Called only on * clean shutdown when the ring confirms every published FSN has been * acked — at that moment the slot has no recoverable work and the * files are pure noise that would mislead the next sender's recovery. - * Best-effort: logs and continues on failures, since we're already on - * the close path. + *

    + * Removal runs in ascending segment order and STOPS at the first + * failure, so whatever survives (a failure here, or a crash mid-loop) + * is always a contiguous top slice of the ring: recovery's + * FSN-contiguity check still passes, and the retained ack watermark + * (== the final acked FSN == the highest frame on disk) still covers + * every surviving frame, so a successor replays nothing. + * + * @return {@code true} only when enumeration succeeded and every + * {@code .sfa} file was confirmed removed — the caller keeps the ack + * watermark on {@code false} so residual acknowledged segments stay + * covered. */ - private static void unlinkAllSegmentFiles(String dir) { - if (!io.questdb.client.std.Files.exists(dir)) return; + private static boolean unlinkAllSegmentFiles(String dir) { + if (!io.questdb.client.std.Files.exists(dir)) return true; long find = io.questdb.client.std.Files.findFirst(dir); if (find < 0) { LOG.warn("close-time unlink could not enumerate {}; " + "any residual sf-*.sfa files will be picked up by the next recovery", dir); - return; + return false; } - if (find == 0) return; + if (find == 0) return true; + ArrayList names = new ArrayList<>(); + int rc = 1; try { - int rc = 1; while (rc > 0) { String name = io.questdb.client.std.Files.utf8ToString( io.questdb.client.std.Files.findName(find)); rc = io.questdb.client.std.Files.findNext(find); - if (name == null || !name.endsWith(".sfa")) continue; - String path = dir + "/" + name; - if (!io.questdb.client.std.Files.remove(path)) { - LOG.warn("Failed to unlink fully-acked segment {} on close", path); + if (name != null && name.endsWith(".sfa")) { + names.add(name); } } } finally { io.questdb.client.std.Files.findClose(find); } + if (rc < 0) { + // A partial listing must not drive any unlink: removing only the + // files we happened to see could delete the segment holding the + // highest frame while a lower one survives, leaving residual + // state the retained watermark can no longer vouch for. + LOG.warn("close-time unlink could not fully enumerate {}; " + + "leaving all segment files for the next recovery", dir); + return false; + } + names.sort((a, b) -> { + int byRank = Long.compare(segmentCleanupRank(a), segmentCleanupRank(b)); + return byRank != 0 ? byRank : a.compareTo(b); + }); + for (int i = 0, n = names.size(); i < n; i++) { + String path = dir + "/" + names.get(i); + if (!io.questdb.client.std.Files.remove(path)) { + LOG.warn("Failed to unlink fully-acked segment {} on close; stopping so the " + + "residual files stay a contiguous, watermark-covered range", path); + return false; + } + } + return true; } } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java index 13a69f77..32ec4b30 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoop.java @@ -200,7 +200,7 @@ public final class CursorWebSocketSendLoop implements QuietCloseable { // by category. Includes both retriable and terminal outcomes — i.e. every // server-side rejection observed regardless of how the loop reacted. private final AtomicLong totalServerErrors = new AtomicLong(); - private WebSocketClient client; + private volatile WebSocketClient client; // Optional: when non-null, every server-rejection error (retriable and // terminal alike) is offered to the dispatcher for async delivery to the user's // handler. Null disables async delivery entirely; the producer-side @@ -763,6 +763,25 @@ public synchronized void close() { // would block forever. isAlive()==false also covers the normal // post-exit case where the latch is already counted down. if (t.isAlive()) { + // Break a native send/receive before joining. Full client close + // must remain after the worker exit because it frees buffers the + // worker may still access. + WebSocketClient activeClient = client; + if (activeClient != null) { + try { + activeClient.closeTraffic(); + } catch (Throwable e) { + // A custom transport that cannot safely cancel traffic + // must not be destructively closed while the worker is + // live. Fail without joining indefinitely; the worker's + // exit path retains ownership of final cleanup. + throw new LineSenderException( + "cursor I/O thread did not stop: active transport does not support safe traffic shutdown; " + + "client/engine teardown is delegated to the I/O thread's exit path", + e + ); + } + } try { shutdownLatch.await(); } catch (InterruptedException e) { @@ -1114,7 +1133,10 @@ private void clearDurableAckTracking() { * before the first connect attempt — see {@link #failPaced}. */ private void connectLoop(Throwable initial, String phase, long paceFirstAttemptMillis) { - if (reconnectFactory == null || !running) { + if (!running) { + return; + } + if (reconnectFactory == null) { recordFatal(initial); return; } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java index 3b7cff60..578c4f07 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentManager.java @@ -34,7 +34,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReferenceFieldUpdater; import java.util.concurrent.locks.LockSupport; /** @@ -66,10 +68,19 @@ public final class SegmentManager implements QuietCloseable { public static final long DEFAULT_POLL_NANOS = 1_000_000L; // 1 ms public static final long DISK_FULL_LOG_THROTTLE_NANOS = 30_000_000_000L; // 30 s public static final long UNLIMITED_TOTAL_BYTES = Long.MAX_VALUE; + private static final int ENTRY_DEREGISTERED = 3; + private static final int ENTRY_DEREGISTERED_IN_SERVICE = 2; + private static final int ENTRY_IN_SERVICE = 1; + private static final int ENTRY_REGISTERED = 0; + private static final AtomicReferenceFieldUpdater ENTRY_CLEANUP_UPDATER = + AtomicReferenceFieldUpdater.newUpdater(RingEntry.class, Runnable.class, "quiescenceCleanup"); + private static final AtomicIntegerFieldUpdater ENTRY_STATE_UPDATER = + AtomicIntegerFieldUpdater.newUpdater(RingEntry.class, "state"); private static final Logger LOG = LoggerFactory.getLogger(SegmentManager.class); private static final long WORKER_JOIN_TIMEOUT_MILLIS = 5_000L; private final AtomicLong fileGeneration = new AtomicLong(); + private final FilesFacade filesFacade; private final Object lock = new Object(); private final long maxTotalBytes; // Reused by the manager worker thread to build spare-segment paths @@ -86,6 +97,17 @@ public final class SegmentManager implements QuietCloseable { private final ObjList ringSnapshot = new ObjList<>(); private final ObjList rings = new ObjList<>(); private final long segmentSizeBytes; + // Test seam: runs after a deferred ring-pass cleanup returns. Null in + // production; public sender/pool lifecycle tests use it to observe exact + // callback completion without sleeps or polling. + private volatile Runnable afterRingCleanupHook; + // Test seam: runs at the top of deferUntilWorkerExit, before the + // worker-liveness check. Null in production; registration-failure tests + // throw from it to simulate an allocation failure (OOM building the + // cleanup lambda or growing exitCleanups) while the worker is still + // live. Callers must treat such a throw as "worker state unknown", + // never as the exact false return meaning the worker loop has exited. + private volatile Runnable beforeExitCleanupRegistrationHook; // Test seam: runs on the worker thread just before the install path's // synchronized(lock) entry (the one that performs installHotSpare + the // totalBytes += segmentSize commit). Null in production; tests use it to @@ -93,33 +115,70 @@ public final class SegmentManager implements QuietCloseable { // but before ownership/accounting commit. Callers may inject a deregister // or hold this stale worker snapshot while caller-side cleanup runs. private volatile Runnable beforeInstallSyncHook; + // Test seam: records entry into the per-ring quiescence wait. Null in + // production; owned-engine close tests use it to prove they take only the + // stronger whole-manager join path, not two sequential timeout budgets. + private volatile Runnable beforeRingQuiescenceAwaitHook; // Test seam: runs on the worker thread just before the trim block's // synchronized(lock) entry. Null in production; only // SegmentManagerTrimDeregisterRaceTest installs it, to deterministically // inject a deregister(ring) call into the exact race window that the - // registered flag check inside the trim block closes for watermark writes - // and totalBytes accounting. + // entry-state check inside the trim block closes for watermark writes and + // totalBytes accounting. private volatile Runnable beforeTrimSyncHook; + // Entry the worker is claiming or servicing, or null between passes. + // Volatile publication lets teardown find the entry after deregister() + // removes it from rings. RingEntry.state is the authoritative barrier: + // the worker publishes this reference before its REGISTERED->IN_SERVICE + // CAS and only touches ring resources after that CAS succeeds. + private volatile RingEntry inService; + // Cleanup actions handed to the worker's exit block by an owning engine + // whose close() found the worker still mid service pass after the bounded + // join timed out (see deferUntilWorkerExit). Guarded by {@link #lock}; + // consumed exactly once by the worker-loop finally, which runs them + // OUTSIDE `lock`: they perform syscalls (munmap/unlink/flock release), + // and no caller-facing lock may ever be held while running third-party + // cleanup code. + private ObjList exitCleanups; + // A private manager belongs to exactly one CursorSendEngine. Its callback + // is preallocated by that engine and stored directly here, so the critical + // timed-out-close handoff never allocates an ObjList or grows a backing + // array. Guarded by lock and consumed on worker-loop exit outside lock. + private Runnable ownedEngineExitCleanup; private long lastDiskFullLogNs; private volatile boolean running; + // pathScratch free-exactly-once coordination between a timed-out close() + // and the worker's exit path. All three are guarded by {@link #lock}. + // When close() gives up on the join while the worker loop has not yet + // exited, it hands scratch ownership to the worker + // (scratchHandedToWorker=true) and the worker frees the buffer in its + // exit block; in every other case close() frees it. Without the handoff, + // a worker that outlives the bounded join leaks the native scratch + // buffer forever, because nobody retries manager cleanup after close() + // returns. + private boolean scratchFreed; + private boolean scratchHandedToWorker; + private boolean workerLoopExited; // Total bytes currently allocated across every segment owned by every // registered ring (active + sealed + hot-spare). Mutated by the manager // thread on provision/trim and by register/deregister callers under // {@link #lock}; the lock covers both paths so the counter stays // consistent across registration boundaries. private long totalBytes; - private long workerJoinTimeoutMillis = WORKER_JOIN_TIMEOUT_MILLIS; + // volatile: read by awaitRingQuiescence() from arbitrary caller threads + // while the @TestOnly setter may run on another. + private volatile long workerJoinTimeoutMillis = WORKER_JOIN_TIMEOUT_MILLIS; // volatile because wakeWorker() reads workerThread without holding the // monitor; the synchronized start()/close() pair handles the // start-vs-close ordering. private volatile Thread workerThread; public SegmentManager(long segmentSizeBytes) { - this(segmentSizeBytes, DEFAULT_POLL_NANOS, UNLIMITED_TOTAL_BYTES); + this(segmentSizeBytes, DEFAULT_POLL_NANOS, UNLIMITED_TOTAL_BYTES, FilesFacade.INSTANCE); } public SegmentManager(long segmentSizeBytes, long pollNanos) { - this(segmentSizeBytes, pollNanos, UNLIMITED_TOTAL_BYTES); + this(segmentSizeBytes, pollNanos, UNLIMITED_TOTAL_BYTES, FilesFacade.INSTANCE); } /** @@ -145,6 +204,11 @@ public SegmentManager(long segmentSizeBytes, long pollNanos) { * hold an initial active plus one hot spare. */ public SegmentManager(long segmentSizeBytes, long pollNanos, long maxTotalBytes) { + this(segmentSizeBytes, pollNanos, maxTotalBytes, FilesFacade.INSTANCE); + } + + @TestOnly + public SegmentManager(long segmentSizeBytes, long pollNanos, long maxTotalBytes, FilesFacade filesFacade) { // The pathScratch field initializer has already allocated its native // buffer by the time this body runs, so a validation throw must free // it or every failed construction leaks 256 bytes of native memory @@ -159,6 +223,7 @@ public SegmentManager(long segmentSizeBytes, long pollNanos, long maxTotalBytes) "maxTotalBytes (" + maxTotalBytes + ") must allow at least one segment of " + segmentSizeBytes + " bytes"); } + this.filesFacade = filesFacade; this.segmentSizeBytes = segmentSizeBytes; this.pollNanos = pollNanos; this.maxTotalBytes = maxTotalBytes; @@ -194,18 +259,210 @@ public synchronized void close() { } } if (t.isAlive()) { - LOG.warn("SegmentManager worker did not stop before close wait completed; " - + "leaving worker-owned resources allocated"); - return; + synchronized (lock) { + if (!workerLoopExited) { + // Hand pathScratch ownership to the worker: its exit + // block frees the buffer under the same lock, so the + // native allocation is reclaimed even though this + // close() could not confirm termination. workerThread + // stays set so isWorkerReaped() reports the incomplete + // shutdown and a later close() can retry the join. + scratchHandedToWorker = true; + LOG.warn("SegmentManager worker did not stop before close wait completed; " + + "worker frees its native scratch buffer on exit"); + return; + } + } + // The worker loop has already run its exit block; the thread + // is at most a few instructions from terminating and can no + // longer touch manager state. Fall through and reap it. } workerThread = null; } // Free the rotation-path native scratch buffer only after worker - // termination has been observed. The worker is the only thread that - // touches the buffer, but close() uses a bounded join; if the worker is - // still alive, leaking this one scratch allocation is safer than freeing - // native memory it may still read or write. - pathScratch.close(); + // termination (or worker-loop exit) has been observed. The worker is + // the only thread that touches the buffer; the scratchFreed flag + // (shared with the worker's exit block) makes the free exactly-once + // no matter which side runs last. + synchronized (lock) { + if (!scratchFreed) { + scratchFreed = true; + pathScratch.close(); + } + } + } + + /** + * Hands the single owning engine's preallocated close callback to the + * worker exit block. Registration only assigns a reference under + * {@link #lock}; it cannot allocate in the timed-out teardown path. + * Returns {@code false} only after the worker loop has exited (or when it + * never started), so the caller may then clean up inline safely. + */ + public boolean deferOwnedEngineCloseUntilWorkerExit(Runnable cleanup) { + synchronized (lock) { + if (workerLoopExited || workerThread == null) { + return false; + } + if (ownedEngineExitCleanup != null && ownedEngineExitCleanup != cleanup) { + throw new IllegalStateException("owned manager already has an engine-exit cleanup"); + } + ownedEngineExitCleanup = cleanup; + return true; + } + } + + /** + * Hands a cleanup action to the current service pass for {@code ring}. + * The worker runs it outside the manager lock immediately after that pass + * finishes. Returns {@code false} when no pass for the ring remains in + * flight, in which case the caller already owns a quiescent ring and must + * run the cleanup itself. + *

    + * Registration and pass completion coordinate through the entry's atomic + * state and callback fields, so there is no gap without an owner: either + * this method attaches the cleanup while the pass remains active, the + * worker claims an attached cleanup while completing, or this method + * observes completed state and rejects the handoff. A repeated registration + * for the same pass returns {@code true} without replacing its existing + * owner. + */ + public boolean deferUntilRingQuiescent(SegmentRing ring, Runnable cleanup) { + RingEntry e = inService; + if (e == null || e.ring != ring || !e.isInService()) { + return false; + } + Runnable existing = ENTRY_CLEANUP_UPDATER.get(e); + if (existing == null && ENTRY_CLEANUP_UPDATER.compareAndSet(e, null, cleanup)) { + if (e.isInService()) { + return true; + } + // Completion changed the state before observing the callback. + // Remove our callback and clean inline, unless the worker already + // claimed it (in which case that worker remains the owner). + return !ENTRY_CLEANUP_UPDATER.compareAndSet(e, cleanup, null); + } + // A callback already attached to this pass is an owner. The sole + // production caller always supplies the engine's preallocated bound + // callback; duplicates intentionally do not replace it. + return true; + } + + /** + * Hands a cleanup action to the worker thread's exit block, to run + * strictly after the worker loop has finished its final service pass -- + * i.e. after the last point where the worker can create, write or unlink + * anything under a registered ring's slot directory. Returns {@code false} + * when the worker loop has already exited (or the worker never started): + * the caller must run the cleanup itself, which is equally safe for the + * same reason -- no further worker access to the slot is possible. + *

    + * This is the slot-ownership transfer used by an owning engine's close() + * when the bounded worker join timed out: instead of retiring the slot + * until process exit, ring/watermark/flock release moves to the worker, + * which is provably the last thread able to touch the slot directory. + * The registration here and the exit block's {@code workerLoopExited} + * flip share {@link #lock}, so the cleanup runs exactly once: either it + * is registered before the flip and the worker runs it, or the flip won + * and this method rejects the handoff. + *

    + * May throw on allocation failure (the lambda at the call site, the + * {@code ObjList}, or its growth). A throw carries NO liveness + * information: the worker was never observed, so the caller must treat + * it as "worker possibly still live" and retain resources — never as + * the exact {@code false} return above. + */ + public boolean deferUntilWorkerExit(Runnable cleanup) { + Runnable hook = beforeExitCleanupRegistrationHook; + if (hook != null) { + hook.run(); + } + synchronized (lock) { + if (workerLoopExited || workerThread == null) { + return false; + } + if (exitCleanups == null) { + exitCleanups = new ObjList<>(); + } + exitCleanups.add(cleanup); + return true; + } + } + + /** + * Quiescence barrier for {@link #deregister(SegmentRing)}. Blocks until + * the worker thread is provably no longer executing a service pass for + * {@code ring}, or the worker-join timeout elapses. After this returns + * {@code true}, the worker will never again touch the ring, its + * watermark, or path names under its slot directory: deregister has + * removed the entry from the registry, a stale snapshot entry that has + * not started its pass is skipped by the registration check at the top + * of {@link #serviceRing(RingEntry)}, and this method has observed the + * end of any in-flight pass. Only then may the caller release dependent + * resources (ring, watermark, segment files, slot lock). + *

    + * Returns {@code true} immediately when no worker is running or when + * called from the worker thread itself (test hooks inject deregister + * calls there; waiting would self-deadlock). Returns {@code false} when + * the in-flight pass did not finish within the timeout — the caller must + * treat the worker as still live and leak rather than release. + *

    + * A pending caller interrupt is preserved but does not abort the wait, + * mirroring {@link #close()}. + */ + public boolean awaitRingQuiescence(SegmentRing ring) { + Runnable hook = beforeRingQuiescenceAwaitHook; + if (hook != null) { + hook.run(); + } + Thread t = workerThread; + if (t == null || t == Thread.currentThread()) { + return true; + } + long deadlineNanos = System.nanoTime() + workerJoinTimeoutMillis * 1_000_000L; + boolean interrupted = Thread.interrupted(); + try { + RingEntry e = inService; + if (e == null || e.ring != ring || !e.isInService()) { + return true; + } + synchronized (lock) { + e.quiescenceWaiters++; + try { + while (e.isInService()) { + long remainingNanos = deadlineNanos - System.nanoTime(); + if (remainingNanos <= 0) { + return false; + } + try { + // Round up so a sub-millisecond remainder still waits + // instead of spinning through wait(0) == wait-forever. + lock.wait(Math.max(1L, remainingNanos / 1_000_000L)); + } catch (InterruptedException ignored) { + interrupted = true; + } + } + } finally { + e.quiescenceWaiters--; + } + } + return true; + } finally { + if (interrupted) { + Thread.currentThread().interrupt(); + } + } + } + + /** + * True when no manager worker thread can be running: either + * {@link #start()} was never called, or a {@link #close()} confirmed + * worker termination and reaped the thread. Owners use this as a + * stronger fallback barrier when {@link #awaitRingQuiescence(SegmentRing)} + * times out but a subsequent {@code close()} join succeeded. + */ + public synchronized boolean isWorkerReaped() { + return workerThread == null; } /** @@ -213,6 +470,13 @@ public synchronized void close() { * created after this returns, but already-installed spares stay with * the ring (the ring closes them on its own {@link SegmentRing#close}). * Idempotent; safe to call from any thread. + *

    + * Non-blocking: a worker service pass already in flight for this ring + * may still be running when this returns. Callers about to release + * resources the worker can reach (the ring itself, its watermark, its + * segment files, or the slot lock guarding its directory) MUST follow + * up with {@link #awaitRingQuiescence(SegmentRing)} and only release on + * a {@code true} result. */ public void deregister(SegmentRing ring) { synchronized (lock) { @@ -226,7 +490,7 @@ public void deregister(SegmentRing ring) { // single subtraction covers both the initial seed // and the net manager activity (provisions minus // trims) for this ring. - e.registered = false; + e.deregister(); totalBytes -= ring.totalSegmentBytes(); rings.remove(i); return; @@ -297,11 +561,26 @@ public void register(SegmentRing ring, String dir, AckWatermark watermark) { wakeWorker(); } + @TestOnly + public void setAfterRingCleanupHook(Runnable hook) { + this.afterRingCleanupHook = hook; + } + + @TestOnly + public void setBeforeExitCleanupRegistrationHook(Runnable hook) { + this.beforeExitCleanupRegistrationHook = hook; + } + @TestOnly public void setBeforeInstallSyncHook(Runnable hook) { this.beforeInstallSyncHook = hook; } + @TestOnly + public void setBeforeRingQuiescenceAwaitHook(Runnable hook) { + this.beforeRingQuiescenceAwaitHook = hook; + } + @TestOnly public void setBeforeTrimSyncHook(Runnable hook) { this.beforeTrimSyncHook = hook; @@ -341,21 +620,19 @@ public void wakeWorker() { * files in {@code dir}, or {@code -1} if none exist. Skips files that * don't match the pattern (e.g. the legacy {@code sf-initial.sfa}). */ - private static long scanMaxGeneration(String dir) { + private long scanMaxGeneration(String dir) { long max = -1L; - if (!Files.exists(dir)) return max; - long find = Files.findFirst(dir); + if (!filesFacade.exists(dir)) return max; + long find = filesFacade.findFirst(dir); if (find < 0) { - LOG.warn("scanMaxGeneration could not enumerate {}; " - + "next spare may collide with an existing on-disk segment", dir); - return max; + throw new IllegalStateException("could not enumerate SF segment directory " + dir); } if (find == 0) return max; try { int rc = 1; while (rc > 0) { - String name = Files.utf8ToString(Files.findName(find)); - rc = Files.findNext(find); + String name = Files.utf8ToString(filesFacade.findName(find)); + rc = filesFacade.findNext(find); if (name == null || !name.startsWith("sf-") || !name.endsWith(".sfa")) { continue; } @@ -368,8 +645,11 @@ private static long scanMaxGeneration(String dir) { // sf-initial.sfa or non-hex — skip } } + if (rc < 0) { + throw new IllegalStateException("could not fully enumerate SF segment directory " + dir); + } } finally { - Files.findClose(find); + filesFacade.findClose(find); } return max; } @@ -412,6 +692,48 @@ private String nextSparePath(String dir) { } private void serviceRing(RingEntry e) { + // Publish before the CAS so deregister + await can always find the + // entry. The state CAS is the ownership decision: if deregister won, + // this stale snapshot pass skips the ring without touching it. + inService = e; + if (!ENTRY_STATE_UPDATER.compareAndSet(e, ENTRY_REGISTERED, ENTRY_IN_SERVICE)) { + inService = null; + return; + } + try { + serviceRing0(e); + } finally { + e.finishService(); + Runnable cleanup = e.quiescenceCleanup == null + ? null + : ENTRY_CLEANUP_UPDATER.getAndSet(e, null); + inService = null; + // A normal pass performs only the two state CAS operations above. + // The manager monitor is entered here only for an actual close + // waiter; the recheck under lock prevents lost wakeups. + if (e.quiescenceWaiters > 0) { + synchronized (lock) { + if (e.quiescenceWaiters > 0) { + lock.notifyAll(); + } + } + } + if (cleanup != null) { + try { + cleanup.run(); + } catch (Throwable t) { + LOG.error("deferred engine cleanup failed after manager-worker ring pass", t); + } finally { + Runnable hook = afterRingCleanupHook; + if (hook != null) { + hook.run(); + } + } + } + } + } + + private void serviceRing0(RingEntry e) { // 1. Provision a hot spare if the ring needs one AND we have headroom // under the disk-total cap. Cap check is per-tick; if we're capped // here, the ring stays in BACKPRESSURE_NO_SPARE until trim (step 2) @@ -455,7 +777,7 @@ private void serviceRing(RingEntry e) { // via its long-ptr overload, bypassing the byte[] + native // malloc that the String overload would incur on every // rotation. - spare = MmapSegment.create(FilesFacade.INSTANCE, + spare = MmapSegment.create(filesFacade, pathScratch.ptr(), path, e.ring.nextSeqHint(), segmentSizeBytes); } @@ -476,7 +798,7 @@ private void serviceRing(RingEntry e) { // observe the spare in the ring (and subtract it) or // run before installation (so no install happens). synchronized (lock) { - if (e.registered) { + if (e.isRegistered()) { e.ring.installHotSpare(spare); totalBytes += segmentSizeBytes; installed = true; @@ -501,7 +823,7 @@ private void serviceRing(RingEntry e) { // on disk, this is the second-line defense. Repeated // unlink on an already-removed path is a harmless no-op. if (path != null) { - Files.remove(path); + filesFacade.remove(path); } } } @@ -514,83 +836,123 @@ private void serviceRing(RingEntry e) { // The watermark write and totalBytes commit are registration-gated // under `lock` so stale worker snapshots cannot touch the // engine-owned watermark or mutate accounting after deregister() - // returns. drainTrimmable still runs for stale snapshots: it - // transfers ownership of fully-acked sealed segments to this - // worker, preserving the old close + unlink behavior. + // returns. Trimming still runs for stale snapshots: a successful + // close + unlink transfers the fully-acked segment out of the ring, + // preserving cleanup without touching deregistered accounting. // // munmap + unlink stay outside the lock — they can be slow // and shouldn't block register/deregister or sibling rings. - ObjList trim; Runnable hook = beforeTrimSyncHook; if (hook != null) { hook.run(); } + boolean trimFailed = false; + while (true) { + MmapSegment segment = e.ring.firstTrimmable(); + if (segment == null) { + break; + } + String path = segment.path(); + try { + segment.close(); + if (path != null && !filesFacade.remove(path)) { + trimFailed = true; + LOG.warn("Failed to unlink trimmed segment {}", path); + break; + } + synchronized (lock) { + if (e.ring.removeTrimmable(segment) && e.isRegistered()) { + totalBytes -= segment.sizeBytes(); + } + } + } catch (Throwable t) { + trimFailed = true; + LOG.warn("Failed to trim segment {}", path == null ? "" : path, t); + break; + } + } + // An unlink failure leaves the acknowledged segment in the ring and + // on disk. Do not advance the persisted watermark past it: recovery + // must continue to see the failed path as live bookkeeping rather + // than infer that trim committed. A later successful retry persists + // the current cumulative ACK normally. synchronized (lock) { - boolean registered = e.registered; - // Persist the current ackedFsn watermark BEFORE the trim runs. - // On host crash between the persist and the unlinks below, the - // segments survive and the watermark is correct. On crash AFTER - // the unlinks but before next tick, the segments are gone and - // the watermark is stale, but recovery clamps with - // max(lowestSurvivingBaseSeq - 1, watermark) so either ordering - // is safe. Memory-mode rings (and callers that didn't supply a - // watermark) skip the write. - // Persist only on advance to avoid pointless mmap stores when - // ackedFsn is steady. The store is a single 8-byte put against - // an already-mapped region -- no syscall, no allocation -- but - // the gate keeps the dirty-page footprint minimal under - // steady-state load with no new acks arriving. - if (registered && e.watermark != null) { + if (!trimFailed && e.isRegistered() && e.watermark != null) { long currentAck = e.ring.ackedFsn(); if (currentAck > e.lastPersistedAck) { e.watermark.write(currentAck); e.lastPersistedAck = currentAck; } } - trim = e.ring.drainTrimmable(); - if (registered && trim != null) { - for (int i = 0, n = trim.size(); i < n; i++) { - totalBytes -= trim.get(i).sizeBytes(); - } - } - } - if (trim != null) { - for (int i = 0, n = trim.size(); i < n; i++) { - MmapSegment s = trim.get(i); - String path = s.path(); - try { - s.close(); - if (path != null && !Files.remove(path)) { - LOG.warn("Failed to unlink trimmed segment {}", path); - } - } catch (Throwable t) { - LOG.warn("Failed to trim segment {}", path == null ? "" : path, t); - } - } } } private void workerLoop() { - while (running) { - // Snapshot the rings under the lock so we don't hold it through the - // (potentially slow) syscalls during creation/unlink. ringSnapshot - // is a thread-confined field — no per-tick allocation. - ringSnapshot.clear(); + try { + while (running) { + // Snapshot the rings under the lock so we don't hold it through the + // (potentially slow) syscalls during creation/unlink. ringSnapshot + // is a thread-confined field — no per-tick allocation. + ringSnapshot.clear(); + synchronized (lock) { + for (int i = 0, n = rings.size(); i < n; i++) { + ringSnapshot.add(rings.getQuick(i)); + } + } + for (int i = 0, n = ringSnapshot.size(); i < n; i++) { + if (!running) break; + serviceRing(ringSnapshot.getQuick(i)); + } + // Drop strong refs so a deregistered ring becomes collectable + // before the next tick (otherwise the snapshot pins it for up + // to pollNanos after deregister). + ringSnapshot.clear(); + if (!running) break; + LockSupport.parkNanos(pollNanos); + } + } finally { + // If a timed-out close() abandoned the reap, it handed + // pathScratch ownership to this thread (see close()). Freeing it + // here reclaims the native buffer even when the worker outlives + // every close() attempt — nobody else retries manager cleanup. + ObjList cleanups; + Runnable ownedEngineCleanup; synchronized (lock) { - for (int i = 0, n = rings.size(); i < n; i++) { - ringSnapshot.add(rings.getQuick(i)); + workerLoopExited = true; + if (scratchHandedToWorker && !scratchFreed) { + scratchFreed = true; + pathScratch.close(); } + cleanups = exitCleanups; + exitCleanups = null; + ownedEngineCleanup = ownedEngineExitCleanup; + ownedEngineExitCleanup = null; } - for (int i = 0, n = ringSnapshot.size(); i < n; i++) { - if (!running) break; - serviceRing(ringSnapshot.getQuick(i)); + // Deferred engine cleanups run OUTSIDE + // `lock`: they perform syscalls (munmap, unlink, flock release) + // and must never execute under a lock that close()/register/ + // deregister callers contend on. Running them after the loop body + // is what makes the handoff safe: this thread can no longer touch + // any slot path. They must also never block on a caller-held + // monitor — a retried engine.close() joins this thread while + // holding the engine monitor, which is why the engine side uses a + // lock-free claim (CAS), not synchronization, for exactly-once. + if (ownedEngineCleanup != null) { + try { + ownedEngineCleanup.run(); + } catch (Throwable t) { + LOG.error("deferred owned-engine cleanup failed on manager-worker exit", t); + } + } + if (cleanups != null) { + for (int i = 0, n = cleanups.size(); i < n; i++) { + try { + cleanups.getQuick(i).run(); + } catch (Throwable t) { + LOG.error("deferred engine cleanup failed on manager-worker exit", t); + } + } } - // Drop strong refs so a deregistered ring becomes collectable - // before the next tick (otherwise the snapshot pins it for up - // to pollNanos after deregister). - ringSnapshot.clear(); - if (!running) break; - LockSupport.parkNanos(pollNanos); } } @@ -607,16 +969,57 @@ private static final class RingEntry { // Survives across multiple serviceRing ticks and avoids a // write-storm when ackedFsn is steady. long lastPersistedAck = -1L; - // Guarded by SegmentManager.lock. A worker snapshot may retain this - // entry after deregister() removes it from rings; registered=false is - // the O(1) ownership check that prevents post-deregister writes through - // the engine-owned watermark, hot-spare installs, and accounting. - boolean registered = true; + // Updated lock-free by deferUntilRingQuiescent and pass completion. + // The field updater avoids one AtomicReference allocation per ring. + volatile Runnable quiescenceCleanup; + // Waiters mutate this under SegmentManager.lock; pass completion reads + // it before taking the otherwise-cold notification path. + volatile int quiescenceWaiters; + // REGISTERED -> IN_SERVICE -> REGISTERED on a normal pass. + // Deregistration changes either registered state to its corresponding + // deregistered state; completion then changes DEREGISTERED_IN_SERVICE + // to DEREGISTERED. The field updater avoids per-entry allocation. + volatile int state = ENTRY_REGISTERED; RingEntry(SegmentRing ring, String dir, AckWatermark watermark) { this.ring = ring; this.dir = dir; this.watermark = watermark; } + + void deregister() { + while (true) { + int current = state; + int next = current == ENTRY_IN_SERVICE + ? ENTRY_DEREGISTERED_IN_SERVICE + : ENTRY_DEREGISTERED; + if (current == ENTRY_DEREGISTERED || current == ENTRY_DEREGISTERED_IN_SERVICE + || ENTRY_STATE_UPDATER.compareAndSet(this, current, next)) { + return; + } + } + } + + void finishService() { + while (true) { + int current = state; + int next = current == ENTRY_DEREGISTERED_IN_SERVICE + ? ENTRY_DEREGISTERED + : ENTRY_REGISTERED; + if (ENTRY_STATE_UPDATER.compareAndSet(this, current, next)) { + return; + } + } + } + + boolean isInService() { + int current = state; + return current == ENTRY_IN_SERVICE || current == ENTRY_DEREGISTERED_IN_SERVICE; + } + + boolean isRegistered() { + int current = state; + return current == ENTRY_REGISTERED || current == ENTRY_IN_SERVICE; + } } } diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java index c8516c4c..8d929ae1 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SegmentRing.java @@ -516,6 +516,21 @@ public synchronized MmapSegment firstSealed() { return sealedSegments.size() > 0 ? sealedSegments.get(0) : null; } + /** + * Returns the oldest fully acknowledged sealed segment without removing + * it. The segment manager keeps it owned by the ring until close + unlink + * succeeds, so a failed unlink cannot make the path disappear from live + * bookkeeping or allow its identifier to be reused. + */ + public synchronized MmapSegment firstTrimmable() { + if (sealedSegments.size() == 0) { + return null; + } + MmapSegment segment = sealedSegments.get(0); + long lastSeq = segment.baseSeq() + segment.frameCount() - 1; + return lastSeq <= ackedFsn ? segment : null; + } + /** Active segment -- exposed for the I/O thread's "send next batch" path. */ /** * Walks every published frame in the ring (sealed segments plus the active @@ -634,6 +649,22 @@ public long publishedFsn() { return publishedFsn; } + /** + * Commits removal of the segment returned by {@link #firstTrimmable()}. + * Returns false if concurrent lifecycle activity changed the head. + */ + public synchronized boolean removeTrimmable(MmapSegment segment) { + if (sealedSegments.size() == 0 || sealedSegments.get(0) != segment) { + return false; + } + long lastSeq = segment.baseSeq() + segment.frameCount() - 1; + if (lastSeq > ackedFsn) { + return false; + } + sealedSegments.remove(0); + return true; + } + /** * Registers a wakeup callback that the producer thread will invoke when * a hot spare is needed -- either right after a rotation has consumed the diff --git a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java index 0b8379de..2bbcc109 100644 --- a/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java +++ b/core/src/main/java/io/questdb/client/cutlass/qwp/client/sf/cursor/SlotLock.java @@ -36,9 +36,9 @@ * Advisory exclusive lock for a single SF slot directory. *

    * One {@code .lock} file per slot, held via {@code flock}/{@code LockFileEx} - * for the entire lifetime of the engine that owns the slot. The lock is - * automatically released when the fd is closed — including on hard process - * exit, since the kernel cleans up file locks for terminated processes. + * for the entire lifetime of the engine that owns the slot. Normal teardown + * explicitly unlocks it before closing the fd; hard process exit remains a + * backstop because the kernel cleans up file locks for terminated processes. *

    * The holder's PID is written to a sibling {@code .lock.pid} file at * acquisition time. A failed acquisition reads it back so the error message @@ -116,15 +116,44 @@ public String slotDir() { return slotDir; } - @Override - public void close() { - // Closing the fd releases the lock. We do NOT remove the .lock - // file or the .lock.pid sidecar — a stale PID is harmless (next - // acquirer overwrites .lock.pid on success). - if (fd >= 0) { - Files.close(fd); + /** + * Explicitly releases the flock and reports whether the release was + * confirmed. After a successful unlock the native primitive closes + * the descriptor once, best-effort, and this object forgets its numeric + * value. It never retries that close: POSIX leaves descriptor state + * unspecified after some close failures (notably {@code EINTR}), so a + * retry could close an unrelated descriptor that reused the same number. + * We do NOT remove the {@code .lock} file or {@code .lock.pid} sidecar; a + * stale PID is harmless because the next acquirer overwrites it. + *

    + * When the explicit unlock itself fails, the fd is retained so a later + * attempt can safely retry the non-consuming unlock operation. Idempotent + * once the unlock has succeeded. + *

    + * Owners that gate a "slot dir is reusable" signal on the release + * (e.g. {@code CursorSendEngine.finishClose} publishing + * {@code closeCompleted}) must call this and check the result rather + * than {@link #close()}, which is best-effort by contract. + * + * @return {@code true} if the lock was explicitly released (or was already + * released), {@code false} if the OS reported an unlock failure + */ + public synchronized boolean release() { + if (fd < 0) { + return true; + } + if (release0(fd) == 0) { fd = -1; + return true; } + return false; + } + + @Override + public void close() { + // QuietCloseable contract: best-effort, no signal. Callers that + // must confirm the release use release() and check the result. + release(); } private static String readHolder(String pidPath) { @@ -152,6 +181,8 @@ private static String readHolder(String pidPath) { } } + private static native int release0(int fd); + private static void writePid(String pidPath) { long pid; try { diff --git a/core/src/main/java/io/questdb/client/impl/SenderPool.java b/core/src/main/java/io/questdb/client/impl/SenderPool.java index 1cc214b4..e401f864 100644 --- a/core/src/main/java/io/questdb/client/impl/SenderPool.java +++ b/core/src/main/java/io/questdb/client/impl/SenderPool.java @@ -148,6 +148,16 @@ public final class SenderPool implements AutoCloseable { private final Condition slotReleased; // True iff the configuration enables store-and-forward (sf_dir set). private final boolean storeAndForward; + // Test seam: runs immediately before a capacity-starved borrow enters its + // condition wait, while it still holds the pool lock. Null in production; + // concurrency tests use a latch here to prove that several borrowers have + // all reached the wait path before recovering retired capacity. + private volatile Runnable beforeBorrowWaitHook; + // Test seam: runs after a capacity-starved borrow's condition wait has + // exhausted its positive timeout, before the loop's terminal pass. Null in + // production; regression tests release a retired slot here to prove that + // the terminal pass re-probes returned capacity before throwing. + private volatile Runnable borrowWaitExpiredHook; // Slots removed from `all` whose delegate is still releasing its flock. // They keep reserving capacity (and their slotInUse mark) until the // flock drops, so the cap check and the slot allocator stay consistent @@ -171,11 +181,28 @@ public final class SenderPool implements AutoCloseable { // down on another thread. Guarded by lock. private int pendingLeaseTeardowns; // Slots whose delegate close() returned with the SF flock still held - // (the I/O thread refused to stop). Permanently consumed: the index is + // because an I/O or manager worker did not stop. Consumed while retired: // never freed and never reused, so no borrow ever hands out a still- // locked slot dir. Counted in the cap check so the lost capacity is - // accounted for. Guarded by lock; only ever ticks for SF slots. + // accounted for. NOT necessarily permanent: engine cleanup may be pending + // on a worker/I/O-thread exit path, so reprobeRetiredSlots() re-checks + // retiredSlots and returns any index whose flock has since dropped. + // Guarded by lock; only ever ticks for SF slots. private int leakedSlots; + // The retired slots behind the leakedSlots count: runtime reclaim paths + // (discardBroken/reapIdle via reclaimSlot) and the in-range startup- + // recovery pass (recoverOneSlotStep, which retains the recoverer slot for + // exactly this purpose). Re-probed by reprobeRetiredSlots() so a late + // flock release (deferred engine cleanup on a worker exit path) restores + // the pool's capacity instead of ratcheting it down until process exit. + // Out-of-range startup recoverers are NEVER added: they carry no + // leakedSlots tick and their index has no slotInUse entry to free. + // Pre-sized to maxSize (every entry keeps a distinct in-range slot index + // reserved, so size can never exceed maxSize): add() never grows the + // backing array, so a retire (leakedSlots++ then add, under lock) cannot + // fail on allocation and strand a counted-but-untracked slot that + // reprobeRetiredSlots() could never recover. Guarded by lock. + private final ArrayList retiredSlots; // SF slots currently held by the in-range startup-recovery pass // (recoverOneSlotStep): each is reserved under `lock` for the // duration of its drain and counted in the borrow() cap check so a @@ -193,8 +220,9 @@ public final class SenderPool implements AutoCloseable { // races it. recoveryInRangeNext is the next in-range index in [0, maxSize) // for pass 1; recoveryOutOfRange / recoveryOutOfRangeNext are the lazily // built pass-2 work list (same-base slots at index >= maxSize) and its - // cursor; recoveryComplete latches true when the whole scan finishes or is - // aborted, making runStartupRecoveryStep()/...ToCompletion() idempotent. + // cursor; recoveryComplete latches true only when the whole scan finishes. + // A transient build failure or drain timeout leaves the current candidate + // pending so a later tick or explicit drive can retry it on the same pool. private int recoveryInRangeNext; private IntList recoveryOutOfRange; private int recoveryOutOfRangeNext; @@ -293,6 +321,7 @@ public SenderPool( this.maxLifetimeMillis = maxLifetimeMillis; this.all = new ArrayList<>(maxSize); this.available = new ArrayDeque<>(maxSize); + this.retiredSlots = new ArrayList<>(maxSize); this.slotReleased = lock.newCondition(); // Probe the config once, up front: this validates it eagerly (so a // bad config fails at construction even when minSize == 0) and tells @@ -399,8 +428,9 @@ void runStartupRecoveryToCompletion() { * No-op (returns {@code false}) when SF is off, the pool is shutting down, or * recovery has already finished. * - * @return {@code true} if recovery has more work (call again), {@code false} - * when recovery is complete or the pool is shutting down + * @return {@code true} if recovery has more work immediately; {@code false} + * when recovery is complete, the pool is shutting down, or a transient + * failure deferred the current candidate until a later tick */ boolean runStartupRecoveryStep() { if (!storeAndForward || closed || recoveryComplete) { @@ -441,9 +471,10 @@ boolean runStartupRecoveryStep() { * {@code slotInUse} entry and are never allocated by borrow(). *

    * Best-effort throughout: a build/close Error or a slow drain is logged and - * never propagates, since the data stays durable on disk for a later attempt; - * the first build failure or drain timeout latches {@code recoveryComplete} - * (the failure will very likely repeat for every remaining slot). + * never propagates, since the data stays durable on disk for a later attempt. + * A build failure or drain timeout stops the current drive but leaves its + * candidate pending so a later housekeeper tick can retry after a transient + * condition clears; it does not poison recovery for the life of the pool. *

    * Boundedness / residual window. Recovery is driven on the * PoolHousekeeper thread, and {@code close()} relies on a step finishing @@ -470,7 +501,7 @@ private boolean recoverOneSlotStep(long stepBudgetMillis) { recoveryComplete = true; return false; } - final boolean[] flockHeld = new boolean[1]; + final SenderSlot[] retained = new SenderSlot[1]; // Pass 1: in-range managed slots [0, maxSize). Skip live and empty slots // cheaply; spend the step on the first slot that actually holds data. @@ -513,26 +544,31 @@ private boolean recoverOneSlotStep(long stepBudgetMillis) { recoveryInRangeNext++; continue; } - // A real candidate -> spend the step on it. Advance the cursor first - // so a resume never reprocesses this index. - recoveryInRangeNext++; - boolean stopScan = drainCandidateSlotForRecovery(i, slotPath, stepBudgetMillis, flockHeld); + // A real candidate -> spend the step on it. Advance the cursor only + // after success so a transient build/drain failure remains retryable. + boolean stopScan = drainCandidateSlotForRecovery(i, slotPath, stepBudgetMillis, retained); lock.lock(); try { // Release the recovery reservation accounting; from here either // leakedSlots (retire) or the freed index carries the cap math. recoveringSlots--; - if (flockHeld[0]) { - // close() bailed early with the I/O thread still running and - // the flock still held. Retire the slot permanently (mirror + if (retained[0] != null) { + // close() retained the flock because an I/O or manager + // worker did not stop. Retire the slot (mirror // discardBroken/reapIdle): keep slotInUse[i] set and count it // in leakedSlots so the borrow() cap math accounts for the // lost capacity and no later borrow ever reuses the - // still-locked dir. + // still-locked dir. Keep the recoverer in retiredSlots so + // reprobeRetiredSlots() restores the capacity once the + // deferred engine cleanup releases the flock — without it + // the retirement would be permanent even after the release + // (fatal at maxSize=1: every later borrow would time out). leakedSlots++; - LOG.warn("startup SF recovery: slot {} retired permanently: delegate close() returned with " - + "the flock still held (I/O thread refused to stop); pool capacity reduced by 1, " - + "now {} of {} usable [leakedSlots={}]", + retiredSlots.add(retained[0]); + LOG.warn("startup SF recovery: slot {} retired: delegate close() returned with " + + "the flock still held (I/O or manager worker did not stop); pool capacity reduced by 1, " + + "now {} of {} usable [leakedSlots={}]; the slot is re-probed and recovered " + + "if the worker releases the flock later", i, maxSize - leakedSlots, maxSize, leakedSlots); } else { slotInUse[i] = false; @@ -545,12 +581,12 @@ private boolean recoverOneSlotStep(long stepBudgetMillis) { lock.unlock(); } if (stopScan) { - // A build failure or drain timeout that will very likely repeat - // for every remaining slot -- abort the scan; the data stays - // durable on disk for a later attempt. Do not start pass 2. - recoveryComplete = true; + // Stop this drive without advancing the cursor. The same live + // pool retries this candidate after the transient condition is + // removed instead of requiring pool recreation. return false; } + recoveryInRangeNext++; return true; } @@ -571,27 +607,34 @@ private boolean recoverOneSlotStep(long stepBudgetMillis) { if (closed) { return false; } - int idx = recoveryOutOfRange.getQuick(recoveryOutOfRangeNext++); + int idx = recoveryOutOfRange.getQuick(recoveryOutOfRangeNext); String slotPath = sfDir + "/" + slotBaseId + "-" + idx; if (!OrphanScanner.isCandidateOrphan(slotPath)) { + recoveryOutOfRangeNext++; continue; } - boolean stopScan = drainCandidateSlotForRecovery(idx, slotPath, stepBudgetMillis, flockHeld); - if (flockHeld[0]) { + boolean stopScan = drainCandidateSlotForRecovery(idx, slotPath, stepBudgetMillis, retained); + if (retained[0] != null) { // Out of the pool's [0, maxSize) capacity range: there is no // slotInUse entry to retire and no future borrow targets this - // dir, so a still-held flock only leaks this recoverer's I/O - // thread (a best-effort teardown loss, logged). Crucially we do + // dir, so a still-held flock only leaks this recoverer's + // worker-reachable resources (a best-effort teardown loss, + // logged). Crucially we do // NOT touch leakedSlots -- that would wrongly shrink the - // in-range pool capacity. + // in-range pool capacity -- and we do NOT add to retiredSlots: + // there is no capacity to recover, and freeSlotIndex(idx) + // would index past the slotInUse array (sized maxSize). LOG.warn("startup SF recovery: out-of-range slot {} closed with the flock still held " - + "(I/O thread refused to stop); its data is durable on disk for a later attempt", + + "(I/O or manager worker did not stop); its data is durable on disk for a later attempt", slotPath); } if (stopScan) { - recoveryComplete = true; + // Keep the out-of-range cursor on this candidate. In particular, + // a transient flock/build collision must be retried after the + // primary sender returns; no capacity bookkeeping is involved. return false; } + recoveryOutOfRangeNext++; return true; } @@ -605,21 +648,24 @@ private boolean recoverOneSlotStep(long stepBudgetMillis) { * (whose {@link #defaultSender} derives the dir {@code -slotIndex}), * drains its unacked data, and closes the delegate. Shared by both recovery * passes -- the in-range pass and the out-of-range pass -- which differ only - * in their slot bookkeeping, handled by the caller via {@code flockHeld}. + * in their slot bookkeeping, handled by the caller via {@code retainedOut}. * - * @param flockHeld single-element out-param set to {@code true} iff a - * recoverer was built and its {@code close()} returned with - * the flock still held (the I/O thread refused to stop) + * @param retainedOut single-element out-param set to the recoverer iff one + * was built and its {@code close()} returned with the + * flock still held because a worker did not stop; the + * in-range caller keeps it in {@link #retiredSlots} so a + * late flock release can be re-probed. {@code null} when + * the flock was released (or no recoverer was built). * @return {@code true} if a build/drain failure occurred that will very * likely repeat for every remaining slot, so the caller should stop scanning */ private boolean drainCandidateSlotForRecovery(int slotIndex, String slotPath, - long remainingMillis, boolean[] flockHeld) { - flockHeld[0] = false; + long remainingMillis, SenderSlot[] retainedOut) { + retainedOut[0] = null; // Hoisted so the flock check after the try can consult it: // createRecoverer() takes the slot flock on -slotIndex, and - // delegate().close() can early-return with the I/O thread still running - // (flock still held). + // delegate().close() can retain it when an I/O or manager worker does + // not stop. SenderSlot recoverer = null; boolean stopScan = false; try { @@ -638,7 +684,7 @@ private boolean drainCandidateSlotForRecovery(int slotIndex, String slotPath, // likely repeat for every remaining slot, so stop here rather // than pay a connect timeout per slot. LOG.warn("startup SF recovery: could not open slot {} ({}); " - + "skipping remaining slots", slotPath, buildErr.toString()); + + "deferring this and remaining slots", slotPath, buildErr.toString()); return true; } try { @@ -648,7 +694,7 @@ private boolean drainCandidateSlotForRecovery(int slotIndex, String slotPath, // same reasoning as the build-failure case above. if (!recoverer.delegate().drain(remainingMillis)) { LOG.warn("startup SF recovery: drain did not ack slot {} " - + "within {}ms; skipping remaining slots", + + "within {}ms; deferring this and remaining slots", slotPath, remainingMillis); stopScan = true; } @@ -667,8 +713,8 @@ private boolean drainCandidateSlotForRecovery(int slotIndex, String slotPath, LOG.warn("startup SF recovery: scan failed for slot {} ({})", slotPath, scanErr.toString()); } - if (recoverer != null) { - flockHeld[0] = !flockReleased(recoverer); + if (recoverer != null && !flockReleased(recoverer)) { + retainedOut[0] = recoverer; } return stopScan; } @@ -768,12 +814,36 @@ public PooledSender borrow() { created.bumpGeneration(); return new PooledSender(created, created.generation()); } + // Capacity-starved: re-probe retired slots BEFORE the terminal + // timeout check — a deferred engine cleanup may have released a + // flock since the retire, and the freed index can admit a + // creation right now. The delegate normally signals this pool + // after deferred release, while this probe also covers delegates + // that do not expose that notification and release/listener races. + // Ordering matters twice over: a + // zero-timeout (try-once) borrow must get its one probe before + // throwing, and a borrower whose awaitNanos budget just expired + // must get a final probe on its wake-up pass instead of timing + // out on capacity that has already come back. + if (reprobeRetiredSlots()) { + continue; + } if (remainingNanos <= 0) { throw new LineSenderException( "timed out waiting for a Sender from the pool after " + acquireTimeoutMillis + "ms"); } try { + Runnable beforeWaitHook = beforeBorrowWaitHook; + if (beforeWaitHook != null) { + beforeWaitHook.run(); + } remainingNanos = slotReleased.awaitNanos(remainingNanos); + if (remainingNanos <= 0) { + Runnable hook = borrowWaitExpiredHook; + if (hook != null) { + hook.run(); + } + } } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new LineSenderException("interrupted while waiting for a Sender from the pool"); @@ -786,6 +856,16 @@ public PooledSender borrow() { } } + @TestOnly + public void setBeforeBorrowWaitHook(Runnable hook) { + this.beforeBorrowWaitHook = hook; + } + + @TestOnly + public void setBorrowWaitExpiredHook(Runnable hook) { + this.borrowWaitExpiredHook = hook; + } + /** * Raises the shutdown signal early -- without tearing down live delegates -- * so an in-flight startup-recovery step driven on the {@link PoolHousekeeper} @@ -1002,7 +1082,9 @@ private void retireLease(PooledSender ps, String context) { pendingLeaseTeardowns--; if (reserved) { // Free the index only when the flock was released; a slot - // left locked is retired permanently. + // left locked is retired into retiredSlots, recoverable + // by reprobeRetiredSlots() if the deferred cleanup drops + // the flock later. reclaimSlot(s, context); } slotReleased.signalAll(); @@ -1028,6 +1110,11 @@ public void reapIdle() { if (closed) { return; } + // Housekeeper tick doubles as the retired-slot recovery driver: + // a slot retired because its worker did not stop is re-probed + // here and returns to the free set once the deferred cleanup + // finally released its flock. + reprobeRetiredSlots(); Iterator it = available.iterator(); while (it.hasNext() && all.size() > minSize) { SenderSlot s = it.next(); @@ -1067,7 +1154,7 @@ public void reapIdle() { } // Return reserved SF slot indices to the free set -- but only for // slots whose delegate confirmed the flock was released. A slot - // left locked (I/O thread refused to stop) is retired permanently. + // left locked because a worker did not stop is retired permanently. if (storeAndForward) { lock.lock(); try { @@ -1106,12 +1193,15 @@ public int totalSize() { } /** - * Snapshot of the number of SF slots permanently retired because a - * delegate {@code close()} returned with the slot flock still held (the - * I/O thread refused to stop). Each leaked slot permanently lowers the - * pool's effective capacity ({@code maxSize - leakedSlotCount()}). A - * non-zero, growing value explains a pool that has started timing out - * every {@code borrow()}. For metrics and tests. + * Snapshot of the number of SF slots currently retired because a + * delegate {@code close()} returned with the slot flock still held after + * an I/O or manager worker did not stop. Each leaked slot lowers the + * pool's effective capacity ({@code maxSize - leakedSlotCount()}) while + * retired. Retired slots are re-probed (housekeeper tick and + * capacity-starved borrows) and recovered once the delegate's deferred + * cleanup releases the flock, so the count can go back down; a non-zero, + * persistent value means a worker is still wedged and explains a pool + * that has started timing out {@code borrow()}. For metrics and tests. */ public int leakedSlotCount() { lock.lock(); @@ -1123,7 +1213,11 @@ public int leakedSlotCount() { } private SenderSlot createUnlocked(int slotIndex) { - return new SenderSlot(senderFactory.apply(slotIndex), this, slotIndex); + Sender delegate = senderFactory.apply(slotIndex); + if (delegate instanceof QwpWebSocketSender) { + ((QwpWebSocketSender) delegate).setSlotLockReleaseListener(this::recoverReleasedSlots); + } + return new SenderSlot(delegate, this, slotIndex); } /** @@ -1134,7 +1228,11 @@ private SenderSlot createUnlocked(int slotIndex) { * {@link #drainCandidateSlotForRecovery}. */ private SenderSlot createRecoverer(int slotIndex) { - return new SenderSlot(recoverySenderFactory.apply(slotIndex), this, slotIndex); + Sender delegate = recoverySenderFactory.apply(slotIndex); + if (delegate instanceof QwpWebSocketSender) { + ((QwpWebSocketSender) delegate).setSlotLockReleaseListener(this::recoverReleasedSlots); + } + return new SenderSlot(delegate, this, slotIndex); } private Sender defaultSender(int slotIndex) { @@ -1271,7 +1369,7 @@ private void freeSlotIndex(int idx) { * non-QWP delegate never holds an SF flock, so it is always treated as * released. A {@link QwpWebSocketSender} reports it via * {@link QwpWebSocketSender#isSlotLockReleased()} -- false means close() - * bailed early with the I/O thread still running and the flock still held. + * retained the flock because an I/O or manager worker did not stop. */ private static boolean flockReleased(SenderSlot s) { Sender d = s.delegate(); @@ -1281,11 +1379,14 @@ private static boolean flockReleased(SenderSlot s) { /** * Reclaims one SF slot after its delegate's {@code close()} has been * attempted. When the flock was released the index returns to the free - * set; when {@code close()} returned with the flock still held (the I/O - * thread refused to stop) the slot is retired permanently -- - * {@code leakedSlots++} and {@code slotInUse[idx]} stays set -- so the cap - * math accounts for the lost capacity and no later borrow ever reuses the - * still-locked dir. Either way {@code closingSlots} is decremented. + * set; when {@code close()} returned with the flock still held because an + * I/O or manager worker did not stop, the slot is retired -- + * {@code leakedSlots++}, {@code slotInUse[idx]} stays set, and the sender + * joins {@code retiredSlots} -- so the cap math accounts for the lost + * capacity and no borrow reuses the still-locked dir unless + * {@link #reprobeRetiredSlots} later observes the deferred cleanup's + * release and recovers the index. Either way {@code closingSlots} is + * decremented. *

    * Caller must hold {@code lock} and is responsible for signalling waiters * (only the free path admits a new creation). Shared by @@ -1303,9 +1404,62 @@ private boolean reclaimSlot(SenderSlot s, String context) { return true; } leakedSlots++; - LOG.warn("SF slot {} retired permanently{}: delegate close() returned with the flock still held " + - "(I/O thread refused to stop); pool capacity reduced by 1, now {} of {} usable [leakedSlots={}]", + retiredSlots.add(s); + LOG.warn("SF slot {} retired{}: delegate close() returned with the flock still held " + + "(I/O or manager worker did not stop); pool capacity reduced by 1, now {} of {} usable " + + "[leakedSlots={}]; the slot is re-probed and recovered if the worker releases the flock later", s.slotIndex(), context, maxSize - leakedSlots, maxSize, leakedSlots); return false; } + + private void recoverReleasedSlots() { + lock.lock(); + try { + reprobeRetiredSlots(); + } finally { + lock.unlock(); + } + } + + /** + * Re-probes every retired slot (see {@link #reclaimSlot}) and returns to + * the free set any whose delegate now reports the flock released — the + * deferred engine cleanup (manager-worker or I/O-thread exit path) has + * run since the retire. Restores {@code leakedSlots} capacity and signals + * waiters so a parked borrow can admit a creation immediately. + *

    + * Caller must hold {@code lock}. The probe is cheap on the delegate side + * ({@code isSlotLockReleased()} reads volatiles, and only re-arms the + * shared flock-release retry in the rare orphaned-retry state), so + * holding the pool lock across it cannot stall behind delegate teardown. + * + * @return {@code true} if at least one slot's capacity was recovered + */ + private boolean reprobeRetiredSlots() { + boolean recovered = false; + for (int i = retiredSlots.size() - 1; i >= 0; i--) { + SenderSlot s = retiredSlots.get(i); + if (flockReleased(s)) { + // Order is irrelevant. Swap with the tail before removing so + // each recovery does O(1) list work instead of shifting the + // remaining retired entries. The tail has already been probed + // by this reverse scan and, if still present, is unreleased. + int last = retiredSlots.size() - 1; + if (i < last) { + retiredSlots.set(i, retiredSlots.get(last)); + } + retiredSlots.remove(last); + leakedSlots--; + freeSlotIndex(s.slotIndex()); + recovered = true; + LOG.info("SF slot {} recovered: deferred cleanup released the flock after retirement; " + + "pool capacity restored, now {} of {} usable [leakedSlots={}]", + s.slotIndex(), maxSize - leakedSlots, maxSize, leakedSlots); + } + } + if (recovered) { + slotReleased.signalAll(); + } + return recovered; + } } diff --git a/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java b/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java index c1b1eec7..b5e43a35 100644 --- a/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java +++ b/core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java @@ -149,6 +149,14 @@ public void close() { } } + @Override + public void closeTraffic() { + // A concurrent TLS send may still use the SSLEngine and direct buffers. + // Shut down only the delegate traffic path here; close() releases the + // retained fd and frees TLS state after the owning worker has stopped. + delegate.closeTraffic(); + } + @Override public int getFd() { return delegate.getFd(); diff --git a/core/src/main/java/io/questdb/client/network/Net.java b/core/src/main/java/io/questdb/client/network/Net.java index f649d330..b9e669d0 100644 --- a/core/src/main/java/io/questdb/client/network/Net.java +++ b/core/src/main/java/io/questdb/client/network/Net.java @@ -151,6 +151,8 @@ public static void init() { public static native int setTcpNoDelay(int fd, boolean noDelay); + public static native int shutdown(int fd); + public static long sockaddr(int ipv4address, int port) { SOCK_ADDR_COUNTER.incrementAndGet(); return sockaddr0(ipv4address, port); diff --git a/core/src/main/java/io/questdb/client/network/NetworkFacade.java b/core/src/main/java/io/questdb/client/network/NetworkFacade.java index d23824a5..4c66f0df 100644 --- a/core/src/main/java/io/questdb/client/network/NetworkFacade.java +++ b/core/src/main/java/io/questdb/client/network/NetworkFacade.java @@ -78,6 +78,15 @@ public interface NetworkFacade { int setTcpNoDelay(int fd, boolean noDelay); + /** + * Shuts down traffic on a descriptor owned by this facade without releasing + * the descriptor. Custom facades must override this method rather than let + * native code operate on a synthetic or remapped descriptor. + */ + default int shutdown(int fd) { + throw new UnsupportedOperationException("traffic shutdown is not supported by this network facade"); + } + long sockaddr(int address, int port); int socketTcp(boolean blocking); diff --git a/core/src/main/java/io/questdb/client/network/NetworkFacadeImpl.java b/core/src/main/java/io/questdb/client/network/NetworkFacadeImpl.java index 64ea0dc7..1d1d7e20 100644 --- a/core/src/main/java/io/questdb/client/network/NetworkFacadeImpl.java +++ b/core/src/main/java/io/questdb/client/network/NetworkFacadeImpl.java @@ -132,6 +132,11 @@ public int setTcpNoDelay(int fd, boolean noDelay) { return Net.setTcpNoDelay(fd, noDelay); } + @Override + public int shutdown(int fd) { + return Net.shutdown(fd); + } + @Override public long sockaddr(int address, int port) { return Net.sockaddr(address, port); diff --git a/core/src/main/java/io/questdb/client/network/PlainSocket.java b/core/src/main/java/io/questdb/client/network/PlainSocket.java index 555affd2..cc175f51 100644 --- a/core/src/main/java/io/questdb/client/network/PlainSocket.java +++ b/core/src/main/java/io/questdb/client/network/PlainSocket.java @@ -29,7 +29,7 @@ public class PlainSocket implements Socket { private final Logger log; private final NetworkFacade nf; - private int fd = -1; + private volatile int fd = -1; public PlainSocket(NetworkFacade nf, Logger log) { this.nf = nf; @@ -37,13 +37,22 @@ public PlainSocket(NetworkFacade nf, Logger log) { } @Override - public void close() { + public synchronized void close() { if (fd != -1) { nf.close(fd, log); fd = -1; } } + @Override + public synchronized void closeTraffic() { + if (fd != -1 && nf.shutdown(fd) != 0) { + throw new IllegalStateException( + "could not shut down socket traffic [fd=" + fd + ", errno=" + nf.errno() + ']' + ); + } + } + @Override public int getFd() { return fd; diff --git a/core/src/main/java/io/questdb/client/network/Socket.java b/core/src/main/java/io/questdb/client/network/Socket.java index 0cdce517..6fceb30b 100644 --- a/core/src/main/java/io/questdb/client/network/Socket.java +++ b/core/src/main/java/io/questdb/client/network/Socket.java @@ -37,6 +37,17 @@ public interface Socket extends QuietCloseable { int WRITE_FLAG = 1; + /** + * Closes only the network traffic path so a concurrent blocking send or + * receive returns. Implementations must retain any I/O buffers until the + * owning worker has stopped and {@link #close()} performs full cleanup. + * The compatibility default fails without touching implementation-owned + * resources; custom sockets must override this method to opt in. + */ + default void closeTraffic() { + throw new UnsupportedOperationException("traffic shutdown is not supported by this socket"); + } + /** * @return file descriptor associated with the socket. */ diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java index 9f1500f2..e4fb86c5 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/CloseDrainTest.java @@ -35,8 +35,10 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.file.Paths; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; /** * Regression tests for the close() drain semantics. @@ -49,6 +51,83 @@ */ public class CloseDrainTest { + @Test(timeout = 30_000L) + public void testCloseBlocksAcrossAllReplicaWindowUntilPromotion() throws Exception { + DelayingAckHandler handler = new DelayingAckHandler(0); + try (TestWebSocketServer server = new TestWebSocketServer(handler, false, "PRIMARY")) { + server.setRejectWithRole("REPLICA"); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String cfg = "ws::addr=localhost:" + server.getPort() + + ";initial_connect_retry=async" + + ";reconnect_initial_backoff_millis=20" + + ";reconnect_max_backoff_millis=100" + + ";close_flush_timeout_millis=10000;"; + QwpWebSocketSender sender = (QwpWebSocketSender) Sender.fromConfig(cfg); + CountDownLatch closeDrainWaiting = new CountDownLatch(1); + CountDownLatch releaseCloseDrain = new CountDownLatch(1); + AtomicReference closeFailure = new AtomicReference<>(); + AtomicReference hookFailure = new AtomicReference<>(); + sender.setCloseDrainWaitingHook(() -> { + closeDrainWaiting.countDown(); + try { + if (!releaseCloseDrain.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("promotion did not release the close-drain witness"); + } + } catch (Throwable t) { + hookFailure.set(t); + } + }); + sender.table("foo").longColumn("v", 1L).atNow(); + sender.flush(); + + Thread closer = new Thread(() -> { + try { + sender.close(); + } catch (Throwable t) { + closeFailure.set(t); + } + }, "all-replica-close-drain"); + try { + closer.start(); + Assert.assertTrue("server never produced the all-replica role rejection", + server.awaitRoleReject(5, TimeUnit.SECONDS)); + Assert.assertTrue("close never observed its real unacknowledged drain target", + closeDrainWaiting.await(5, TimeUnit.SECONDS)); + + Assert.assertTrue("pre-promotion witness must include a role rejection", + server.roleRejectCount() >= 1); + Assert.assertEquals("pre-promotion data must remain unacknowledged", + -1L, sender.getAckedFsn()); + Assert.assertTrue("close must remain pending for the whole all-replica window", + closer.isAlive()); + Assert.assertEquals("promotion must not have delivered or replayed the frame yet", + 0L, handler.nextSeq.get()); + + // The close-drain barrier has held continuously since it observed + // targetFsn > ackedFsn. Promote first, then release the barrier: + // completion therefore cannot precede the deterministic recovery event. + server.setAdvertisedRole("PRIMARY"); + server.setRejectWithRole(null); + releaseCloseDrain.countDown(); + closer.join(10_000L); + + Assert.assertFalse("close did not complete after promotion", closer.isAlive()); + Assert.assertNull("close-drain witness failed", hookFailure.get()); + Assert.assertNull("close failed after promotion", closeFailure.get()); + Assert.assertEquals("the unacknowledged frame must be delivered exactly once", + 1L, handler.nextSeq.get()); + } finally { + server.setAdvertisedRole("PRIMARY"); + server.setRejectWithRole(null); + releaseCloseDrain.countDown(); + closer.join(10_000L); + sender.close(); + } + } + } + @Test public void testCloseBlocksUntilAckArrives() throws Exception { // Server delays every ACK by 800ms. With the default @@ -77,6 +156,56 @@ public void testCloseBlocksUntilAckArrives() throws Exception { } } + @Test + public void testCloseStartedHookRunsAfterClosedStateTransition() throws Exception { + QwpWebSocketSender sender = QwpWebSocketSender.createForTesting("localhost", 1); + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + AtomicReference closerFailure = new AtomicReference<>(); + AtomicReference hookFailure = new AtomicReference<>(); + sender.setCloseStartedHook(() -> { + try { + try { + sender.table("must_reject_after_close_started"); + Assert.fail("close-started hook ran before closed=true was published"); + } catch (LineSenderException expected) { + // Required lifecycle boundary: public operations already reject. + } + entered.countDown(); + if (!release.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("close-started hook release timed out"); + } + } catch (Throwable t) { + hookFailure.set(t); + entered.countDown(); + } + }); + + Assert.assertEquals("installing the hook must not emit a pre-invocation witness", + 1L, entered.getCount()); + Thread closer = new Thread(() -> { + try { + sender.close(); + } catch (Throwable t) { + closerFailure.set(t); + } + }, "close-started-hook-test"); + try { + closer.start(); + Assert.assertTrue("close() never reached its internal lifecycle witness", + entered.await(10, TimeUnit.SECONDS)); + Assert.assertTrue("close() completed while its internal witness was held", + closer.isAlive()); + } finally { + release.countDown(); + closer.join(10_000L); + sender.close(); + } + Assert.assertFalse("close thread did not finish", closer.isAlive()); + Assert.assertNull("close-started hook failed", hookFailure.get()); + Assert.assertNull("close() failed", closerFailure.get()); + } + @Test public void testCloseFastWhenTimeoutIsZero() throws Exception { // Same delayed-ACK server, but with close_flush_timeout_millis=0 diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderCursorEngineAttachmentTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderCursorEngineAttachmentTest.java new file mode 100644 index 00000000..5bfb0863 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpWebSocketSenderCursorEngineAttachmentTest.java @@ -0,0 +1,141 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client; + +import io.questdb.client.cutlass.line.LineSenderException; +import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; + +import static io.questdb.client.test.tools.TestUtils.assertMemoryLeak; + +public class QwpWebSocketSenderCursorEngineAttachmentTest { + + private static final long SEGMENT_SIZE = 64 * 1024L; + + @Rule + public final TemporaryFolder sfDir = TemporaryFolder.builder().assureDeletion().build(); + + @Test + public void testNullDoesNotDetachAnAttachedEngine() throws Exception { + assertMemoryLeak(() -> { + CursorSendEngine engine = new CursorSendEngine(null, SEGMENT_SIZE); + QwpWebSocketSender sender = QwpWebSocketSender.createForTesting("localhost", 1); + try { + sender.setCursorEngine(engine, true); + assertSecondAttachmentRejected(sender, null, false); + sender.close(); + Assert.assertTrue("sender must retain ownership after rejected detach", + engine.isCloseCompleted()); + } finally { + sender.close(); + engine.close(); + } + }); + } + + @Test + public void testOwnedEngineCannotBeReplacedAndItsSlotIsReleased() throws Exception { + assertMemoryLeak(() -> { + File slotDir = new File(sfDir.getRoot(), "owned-slot"); + CursorSendEngine first = null; + CursorSendEngine replacement = null; + CursorSendEngine reacquired = null; + QwpWebSocketSender sender = null; + boolean replacementRejected = false; + try { + first = new CursorSendEngine(slotDir.getAbsolutePath(), SEGMENT_SIZE); + replacement = new CursorSendEngine(null, SEGMENT_SIZE); + sender = QwpWebSocketSender.createForTesting("localhost", 1); + sender.setCursorEngine(first, true); + try { + sender.setCursorEngine(replacement, true); + } catch (LineSenderException e) { + replacementRejected = true; + Assert.assertTrue(e.getMessage().contains("already attached")); + } + + sender.close(); + sender = null; + try { + reacquired = new CursorSendEngine(slotDir.getAbsolutePath(), SEGMENT_SIZE); + } catch (IllegalStateException e) { + throw new AssertionError("sender close did not release the first owned " + + "engine's slot after a second attachment attempt", e); + } + Assert.assertTrue("second attachment must be rejected", replacementRejected); + } finally { + if (sender != null) { + sender.close(); + } + if (reacquired != null) { + reacquired.close(); + } + if (replacement != null) { + replacement.close(); + } + if (first != null) { + first.close(); + } + } + }); + } + + @Test + public void testSameSharedEngineCannotTransferOwnership() throws Exception { + assertMemoryLeak(() -> { + CursorSendEngine engine = new CursorSendEngine(null, SEGMENT_SIZE); + QwpWebSocketSender sender = QwpWebSocketSender.createForTesting("localhost", 1); + try { + sender.setCursorEngine(engine, false); + assertSecondAttachmentRejected(sender, engine, true); + sender.close(); + Assert.assertFalse("rejected attachment must not transfer ownership", + engine.isCloseCompleted()); + } finally { + sender.close(); + engine.close(); + } + }); + } + + private static void assertSecondAttachmentRejected( + QwpWebSocketSender sender, + CursorSendEngine engine, + boolean takeOwnership + ) { + try { + sender.setCursorEngine(engine, takeOwnership); + Assert.fail("expected second cursor engine attachment to be rejected"); + } catch (LineSenderException e) { + Assert.assertTrue(e.getMessage().contains("already attached")); + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/SlotLockReleasedContractTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/SlotLockReleasedContractTest.java index 85522e9d..f2c560ea 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/SlotLockReleasedContractTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/SlotLockReleasedContractTest.java @@ -24,9 +24,17 @@ package io.questdb.client.test.cutlass.qwp.client; +import io.questdb.client.DefaultHttpClientConfiguration; import io.questdb.client.Sender; +import io.questdb.client.cutlass.http.client.WebSocketClient; import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop; +import io.questdb.client.network.PlainSocketFactory; +import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock; +import io.questdb.client.std.Files; import io.questdb.client.test.cutlass.qwp.websocket.TestWebSocketServer; import io.questdb.client.test.tools.TestUtils; import org.junit.Assert; @@ -36,8 +44,12 @@ import java.lang.reflect.Field; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.file.Paths; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.LockSupport; /** @@ -46,9 +58,13 @@ *

    * The pool's {@code flockReleased(s)} treats {@code false} as "flock still held, * retire the slot permanently". Before this test that contract was driven only @@ -178,6 +194,269 @@ public void testSlotLockNotReleasedWhenIoThreadRefusesToStop() throws Exception }); } + /** + * Manager-worker handoff path: engine close retains the slot while a shared + * manager is still inside a service pass. The sender must expose that + * retained flock to SenderPool. Repeated sender close calls remain no-ops; + * the manager pass owns deferred cleanup and releases the flock when it + * finishes, so the sender can expose recovery without a direct close retry. + */ + @Test(timeout = 30_000L) + public void testSlotLockNotReleasedUntilManagerWorkerQuiesces() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String tmpDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-slot-lock-manager-" + System.nanoTime()).toString(); + Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT)); + String slot = tmpDir + "/slot"; + long segSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L; + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch cleanupFinished = new CountDownLatch(1); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + CursorSendEngine engine = null; + QwpWebSocketSender wss = null; + boolean managerClosed = false; + try { + manager.setAfterRingCleanupHook(cleanupFinished::countDown); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + engine = new CursorSendEngine(slot, segSize, manager); + Assert.assertTrue("worker never reached install hook", + workerBlocked.await(5, TimeUnit.SECONDS)); + + wss = QwpWebSocketSender.createForTesting("localhost", 1); + wss.setCursorEngine(engine, true); + manager.setWorkerJoinTimeoutMillis(50L); + wss.close(); + + Assert.assertFalse("sender reported a retained flock as released", + wss.isSlotLockReleased()); + Assert.assertFalse("engine close must remain incomplete while worker is in service", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("slot became acquirable while manager worker was still in service"); + } catch (Exception expected) { + // good — the incomplete close retained the flock. + } + + // Sender.close() is idempotent: a second call must not retry + // or change ownership while the manager worker is still live. + wss.close(); + Assert.assertFalse("repeated close changed retained-flock reporting", + wss.isSlotLockReleased()); + Assert.assertFalse("repeated close unexpectedly retried engine cleanup", + engine.isCloseCompleted()); + + releaseWorker.countDown(); + Assert.assertTrue("shared-manager pass did not finish deferred cleanup", + cleanupFinished.await(5, TimeUnit.SECONDS)); + Assert.assertTrue("deferred cleanup did not complete after the ring pass", + engine.isCloseCompleted()); + // Recovery contract: the sender re-probes its retained engine, + // so the completed cleanup (and released flock) MUST become + // visible — this is what lets SenderPool recover a slot it + // had already retired as leaked. + Assert.assertTrue("sender must expose the late flock release to the pool", + wss.isSlotLockReleased()); + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after deferred cleanup", probe); + } + + manager.close(); + managerClosed = true; + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + manager.setAfterRingCleanupHook(null); + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + if (wss != null) { + try { + wss.close(); + } catch (Throwable ignored) { + } + } + if (!managerClosed) { + manager.close(); + } + rmDirRecursive(tmpDir); + Files.remove(tmpDir); + } + }); + } + + /** + * Delegated-I/O-close recovery path: when {@code close()} bails early + * because the I/O thread refuses to stop, the engine close is delegated + * to that thread's exit path ({@code delegateEngineClose()} returned + * true) and the sender must retain the engine for re-probing — + * {@code isSlotLockReleased()} stays false while the thread lives, then + * flips true the moment the thread's exit path completes the engine + * close and releases the flock. Pre-fix, this branch discarded the + * engine reference, so the late release was permanently invisible to a + * pool that had retired the slot. The existing forged I/O-refusal test + * can never reach this branch: its sabotaged loop throws from + * {@code delegateEngineClose()} itself (nulled latch), before the + * retained-engine assignment. + *

    + * The wedge is the same deterministic one CursorWebSocketSendLoop's C5 + * test uses: the I/O thread sits in a blocking "connect" (a + * ReconnectFactory immune to unpark, never interrupted by loop.close()), + * and the closer thread's pre-set interrupt flag makes + * {@code shutdownLatch.await()} throw immediately — the real production + * path to {@code ioThreadStopped = false}. + */ + @Test(timeout = 30_000L) + public void testDelegatedIoThreadEngineCloseFlipsSlotLockReleased() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String tmpDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-slot-lock-delegated-" + System.nanoTime()).toString(); + Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT)); + String slot = tmpDir + "/slot"; + long segSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L; + final CountDownLatch enteredConnect = new CountDownLatch(1); + final CountDownLatch releaseConnect = new CountDownLatch(1); + final AtomicReference ioThreadRef = new AtomicReference<>(); + final StubWebSocketClient stubClient = new StubWebSocketClient(); + // Healthy owned manager: once the wedged I/O thread is released, + // its exit-path engine.close() completes normally and releases + // the flock — the flip this test pins. + CursorSendEngine engine = new CursorSendEngine(slot, segSize); + CursorWebSocketSendLoop loop = null; + QwpWebSocketSender wss = null; + try { + // Stand-in for a blocking native connect(2): entered by the + // loop's I/O thread, immune to unpark, never interrupted by + // loop.close(). + CursorWebSocketSendLoop.ReconnectFactory stuckConnect = () -> { + ioThreadRef.set(Thread.currentThread()); + enteredConnect.countDown(); + releaseConnect.await(); + return stubClient; + }; + loop = new CursorWebSocketSendLoop( + null /* async-initial-connect: the I/O thread drives the connect */, + engine, 0L, 1_000L, + stuckConnect, + 5_000L, 100L, 5_000L, false); + loop.start(); + Assert.assertTrue("I/O thread never reached the connect factory", + enteredConnect.await(5, TimeUnit.SECONDS)); + + wss = QwpWebSocketSender.createForTesting("localhost", 1); + wss.setCursorEngine(engine, true); + setField(wss, "cursorSendLoop", loop); + + // Drive the real early-bail close() on a thread whose pending + // interrupt lands in loop.close()'s shutdownLatch.await(). + AtomicReference closeFailure = new AtomicReference<>(); + QwpWebSocketSender wssRef = wss; + Thread closer = new Thread(() -> { + Thread.currentThread().interrupt(); + try { + wssRef.close(); + } catch (Throwable t) { + closeFailure.set(t); + } + }, "delegated-close-closer"); + closer.setDaemon(true); + closer.start(); + closer.join(TimeUnit.SECONDS.toMillis(10)); + Assert.assertFalse("closer thread did not finish", closer.isAlive()); + Assert.assertNotNull("close() must surface the failed I/O-thread stop", + closeFailure.get()); + + // The I/O thread is still wedged: the flock is retained and + // must be reported retained. + Assert.assertFalse( + "isSlotLockReleased() must be false while the delegated engine " + + "close is pending on the wedged I/O thread", + wss.isSlotLockReleased()); + Assert.assertFalse("engine close must not have run yet", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("slot became acquirable while the delegated engine close " + + "was still pending"); + } catch (Exception expected) { + // good — the flock is genuinely held. + } + + // Un-wedge the connect. The I/O thread exits; its exit path + // runs the delegated engine.close(), which releases the flock. + releaseConnect.countDown(); + Thread ioThread = ioThreadRef.get(); + Assert.assertNotNull(ioThread); + ioThread.join(TimeUnit.SECONDS.toMillis(10)); + Assert.assertFalse("I/O thread did not exit after the connect returned", + ioThread.isAlive()); + + // The recovery contract under test: the getter re-probes the + // retained engine, so the late release MUST become visible — + // this is what lets SenderPool recover the retired slot. + Assert.assertTrue("delegated engine close must have completed on the " + + "I/O thread's exit path", + engine.isCloseCompleted()); + Assert.assertTrue( + "isSlotLockReleased() must flip true once the delegated engine " + + "close released the flock — otherwise the pool retires the " + + "slot's capacity until process exit", + wss.isSlotLockReleased()); + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after the delegated close", probe); + } + } finally { + releaseConnect.countDown(); + // Reap the loop's bookkeeping now that the I/O thread is gone + // (close() threw mid-teardown, so ioThread was left set). + Thread.interrupted(); + if (loop != null) { + try { + loop.close(); + } catch (Throwable ignored) { + } + } + if (engine != null && !engine.isCloseCompleted()) { + try { + engine.close(); + } catch (Throwable ignored) { + } + } + // The early-return close() deliberately leaked the resources + // the (then-running) I/O thread might touch; free the same set + // the post-guard tail would have freed. + if (wss != null) { + freeFieldQuietly(wss, "buffer0"); + freeFieldQuietly(wss, "buffer1"); + freeFieldQuietly(wss, "client"); + freeFieldQuietly(wss, "errorDispatcher"); + freeFieldQuietly(wss, "progressDispatcher"); + freeFieldQuietly(wss, "connectionDispatcher"); + } + stubClient.close(); + rmDirRecursive(tmpDir); + Files.remove(tmpDir); + } + }); + } + // ------------------------------------------------------------------ utils private static void freeFieldQuietly(Object target, String name) { @@ -192,6 +471,28 @@ private static void freeFieldQuietly(Object target, String name) { } } + private static void rmDirRecursive(String dir) { + if (!Files.exists(dir)) return; + long find = Files.findFirst(dir); + if (find <= 0) return; + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + String child = dir + "/" + name; + if (!Files.remove(child)) { + rmDirRecursive(child); + Files.remove(child); + } + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } + private static T readField(Object target, String name, Class type) throws Exception { Class cls = target.getClass(); while (cls != null) { @@ -221,6 +522,28 @@ private static void setField(Object target, String name, Object value) throws Ex throw new NoSuchFieldException(name); } + /** + * Minimal concrete {@link WebSocketClient} — never performs I/O. Handed + * to the loop by the stuck-connect factory; the loop's exit path closes + * it (close is idempotent via the superclass). + */ + private static final class StubWebSocketClient extends WebSocketClient { + + StubWebSocketClient() { + super(DefaultHttpClientConfiguration.INSTANCE, PlainSocketFactory.INSTANCE); + } + + @Override + protected void ioWait(int timeout, int op) { + throw new UnsupportedOperationException("stub: no socket"); + } + + @Override + protected void setupIoWait() { + // no-op + } + } + /** ACKs every binary frame with a running sequence so flush/close drain cleanly. */ private static final class AckAllHandler implements TestWebSocketServer.WebSocketServerHandler { private final AtomicLong nextSeq = new AtomicLong(); diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineCloseUnlinkFailureTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineCloseUnlinkFailureTest.java new file mode 100644 index 00000000..a9df2b67 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineCloseUnlinkFailureTest.java @@ -0,0 +1,209 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.cutlass.qwp.client.sf.cursor.AckWatermark; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager; +import io.questdb.client.std.Files; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import io.questdb.client.test.tools.TestUtils; +import org.junit.After; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; + +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +/** + * Regression for the close-time segment cleanup on a fully-drained slot. + *

    + * {@code CursorSendEngine.finishClose} unlinks the acknowledged {@code .sfa} + * files and then removes the ack watermark. When the unlink fails (transient + * I/O error, permission problem), the residual segment files hold rows the + * server already acknowledged. If the watermark does not cover them, a + * successor engine on the same slot seeds recovery from {@code lowestBase - 1} + * and replays every acknowledged row — duplicates on a non-DEDUP table. + *

    + * The test injects the unlink failure by dropping write permission on the + * slot directory (POSIX: unlink requires a writable parent directory), so it + * is skipped on Windows and when permissions are not enforced (root). + * The shared {@link SegmentManager} is deliberately never started: no worker + * thread exists, so no manager tick can persist the watermark behind the + * test's back, and the close-path quiescence barrier is trivially satisfied — + * fully deterministic, no timing coordination needed. + */ +public class CursorSendEngineCloseUnlinkFailureTest { + + private String tmpDir; + + @Before + public void setUp() { + tmpDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-engine-close-unlink-fault-" + System.nanoTime()).toString(); + Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT)); + } + + @After + public void tearDown() { + if (tmpDir != null) { + removeRecursive(tmpDir); + } + } + + @Test(timeout = 20_000L) + public void testFailedCloseTimeUnlinkMustNotExposeAckedFramesToSuccessor() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segmentSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L; + String slot = tmpDir + "/slot"; + Path slotPath = Paths.get(slot); + long payload = Unsafe.malloc(32, MemoryTag.NATIVE_DEFAULT); + SegmentManager manager = new SegmentManager(segmentSize, TimeUnit.SECONDS.toNanos(60)); + CursorSendEngine pred = null; + CursorSendEngine succ = null; + boolean slotDirReadOnly = false; + try { + fill(payload, 32, (byte) 0x33); + pred = new CursorSendEngine(slot, segmentSize, manager); + Assert.assertEquals(0L, pred.appendBlocking(payload, 32)); + Assert.assertEquals(0L, pred.publishedFsn()); + // The server durably acknowledged FSN 0 in this session. + Assert.assertTrue(pred.acknowledge(0L)); + Assert.assertEquals(0L, pred.ackedFsn()); + + // Inject a close-time unlink failure: drop write permission on + // the slot dir. Prove the injection works with a probe file -- + // root (and some filesystems) ignore directory permissions. + String probePath = slot + "/probe"; + Assert.assertTrue(java.nio.file.Files.exists( + java.nio.file.Files.createFile(Paths.get(probePath)))); + try { + setPermissions(slotPath, "r-xr-xr-x"); + } catch (UnsupportedOperationException e) { + Assume.assumeNoException("POSIX permissions unavailable on this platform", e); + } + slotDirReadOnly = true; + boolean probeRemoved = Files.remove(probePath); + if (probeRemoved) { + setPermissions(slotPath, "rwxr-xr-x"); + slotDirReadOnly = false; + } + Assume.assumeFalse("directory permissions not enforced (running as root?)", + probeRemoved); + + // Fully-drained close: tries to unlink the acknowledged + // segment files and fails. + pred.close(); + Assert.assertTrue("flock release needs no dir write; close must complete", + pred.isCloseCompleted()); + pred = null; + Assert.assertTrue("injected unlink failure must leave the acknowledged segment", + Files.exists(slot + "/sf-initial.sfa")); + + // The transient failure clears before the successor arrives. + setPermissions(slotPath, "rwxr-xr-x"); + slotDirReadOnly = false; + Files.remove(probePath); + + succ = new CursorSendEngine(slot, segmentSize, manager); + Assert.assertTrue(succ.wasRecoveredFromDisk()); + Assert.assertEquals(0L, succ.publishedFsn()); + // THE regression: FSN 0 was acknowledged by the server during + // the predecessor's session. The successor must not see it as + // replayable, or a non-DEDUP table receives duplicate rows. + Assert.assertTrue("successor exposes already-acknowledged frames for replay " + + "[ackedFsn=" + succ.ackedFsn() + + ", publishedFsn=" + succ.publishedFsn() + "]", + succ.ackedFsn() >= succ.publishedFsn()); + + // The successor's own fully-drained close retries the cleanup + // now that the failure has cleared: segments and watermark gone. + succ.close(); + Assert.assertTrue(succ.isCloseCompleted()); + succ = null; + Assert.assertFalse("successor close did not retry the segment unlink", + Files.exists(slot + "/sf-initial.sfa")); + Assert.assertFalse("watermark must be removed once no segment file remains", + Files.exists(slot + "/" + AckWatermark.FILE_NAME)); + } finally { + if (slotDirReadOnly) { + try { + setPermissions(slotPath, "rwxr-xr-x"); + } catch (Throwable ignored) { + } + } + if (pred != null) { + pred.close(); + } + if (succ != null) { + succ.close(); + } + manager.close(); + Unsafe.free(payload, 32, MemoryTag.NATIVE_DEFAULT); + } + }); + } + + private static void fill(long address, int len, byte value) { + for (int i = 0; i < len; i++) { + Unsafe.getUnsafe().putByte(address + i, value); + } + } + + private static void removeRecursive(String dir) { + long find = Files.findFirst(dir); + if (find > 0) { + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + String child = dir + "/" + name; + if (!Files.remove(child)) { + removeRecursive(child); + } + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } + Files.remove(dir); + } + + private static void setPermissions(Path path, String posix) throws Exception { + Set perms = PosixFilePermissions.fromString(posix); + java.nio.file.Files.setPosixFilePermissions(path, perms); + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineSlotReacquisitionTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineSlotReacquisitionTest.java new file mode 100644 index 00000000..6c08aeff --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineSlotReacquisitionTest.java @@ -0,0 +1,849 @@ +/******************************************************************************* + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentRing; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock; +import io.questdb.client.std.Files; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import io.questdb.client.test.tools.TestUtils; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.nio.file.Paths; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +/** + * Engine-level regression for the shutdown hazard where + * {@link CursorSendEngine#close()} released the slot lock, closed the ring + * and watermark, and unlinked segment files while the shared + * {@link SegmentManager} worker was still mid service pass for the engine's + * ring. A replacement engine could acquire the same slot the moment the + * lock was released, after which the stale worker's abandon/trim path could + * unlink a segment path the replacement was actively writing through — + * store-and-forward data loss after restart. + *

    + * The fix makes {@code close()} run a quiescence barrier + * ({@link SegmentManager#awaitRingQuiescence}) after {@code deregister} and + * refuse to release any worker-reachable resource (ring, watermark, segment + * files, slot lock) until the barrier confirms the worker cannot touch the + * slot again. On barrier timeout an owned-manager engine hands cleanup + * ownership to the worker's exit path (see + * {@code SegmentManager.deferUntilWorkerExit}); a shared-manager engine + * deliberately leaks and a later {@code close()} retries the cleanup. + */ +public class CursorSendEngineSlotReacquisitionTest { + + private String tmpDir; + + @Before + public void setUp() { + tmpDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-engine-slot-reacq-" + System.nanoTime()).toString(); + Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT)); + } + + @After + public void tearDown() { + if (tmpDir == null) return; + rmDirRecursive(tmpDir); + Files.remove(tmpDir); + } + + /** + * The structural guarantee: while the manager worker is provably still + * inside a service pass for the engine's ring, {@code close()} must NOT + * hand the slot to anyone else. With the quiescence barrier reverted, + * close() releases the slot lock immediately and the mid-test + * {@code SlotLock.acquire} probe succeeds. Once the pass finishes, its + * deferred cleanup must release the slot without a direct close retry. + */ + @Test(timeout = 30_000L) + public void testCloseRetainsSlotWhileWorkerIsMidServicePass() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + 32); + String slot = tmpDir + "/slot"; + // 60 s poll: the worker only acts when explicitly woken, so the + // single pass we park below is the only pass in flight. + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch cleanupFinished = new CountDownLatch(1); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + boolean managerClosed = false; + CursorSendEngine engine = null; + try { + manager.setAfterRingCleanupHook(cleanupFinished::countDown); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + + // Shared manager: ownsManager=false, so engine close() cannot + // fall back on manager.close()'s join — the per-ring barrier + // is the only protection, which is exactly what we pin here. + engine = new CursorSendEngine(slot, segSize, manager); + Assert.assertTrue("worker never reached the install hook", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Barrier must time out fast: the worker is parked inside the + // service pass for this engine's ring. + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse("incomplete close must remain observable to the owner", + engine.isCloseCompleted()); + + // The slot must still be locked: a replacement engine (or raw + // SlotLock) acquiring it now would race the stale worker. + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("engine.close() released the slot lock while the manager " + + "worker was still mid service pass for its ring — a " + + "replacement engine could acquire the slot and have its " + + "segment files unlinked by the stale worker"); + } catch (Exception expected) { + // good — slot retained. + } + + // Let the worker finish its pass (it abandons the spare: the + // ring was deregistered by the close attempt above). + releaseWorker.countDown(); + Assert.assertTrue("ring pass did not finish deferred cleanup", + cleanupFinished.await(5, TimeUnit.SECONDS)); + Assert.assertTrue("ring-pass cleanup must report complete cleanup", + engine.isCloseCompleted()); + engine = null; + + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after a completed close", probe); + } catch (Exception e) { + throw new AssertionError("ring-pass cleanup did not release the slot lock", e); + } + + manager.close(); + managerClosed = true; + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + manager.setAfterRingCleanupHook(null); + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + if (!managerClosed) { + manager.close(); + } + } + }); + } + + /** + * Owned-manager twin of {@link #testCloseRetainsSlotWhileWorkerIsMidServicePass}: + * the ONLY construction shape production uses (Sender.build, BackgroundDrainer, + * QwpWebSocketSender.connect all own their manager). The owned close path does + * not run the per-ring barrier at all — it relies on {@code manager.close()}'s + * bounded join and the {@code isWorkerReaped()} check. If that check regressed + * to report quiescence unconditionally (or {@code isWorkerReaped()} itself + * returned true while the worker is alive), close() would release the slot + * lock mid service pass and the shared-manager tests would stay green — this + * test is the red gate for the production path. + *

    + * Determinism: the owned manager starts inside the engine ctor (1 ms poll), + * so its first spare-install pass races test setup. We first wait until the + * initial hot spare is installed — after that the worker cannot enter another + * install pass until a rotation consumes the spare, so the park hook installed + * afterwards can neither be missed nor fire early. Two appends then fill the + * active segment and rotate onto the spare; the worker's next poll tick + * re-enters the install pass and parks in the hook. + */ + @Test(timeout = 30_000L) + public void testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int payloadLen = 32; + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen); + String slot = tmpDir + "/owned-parked-slot"; + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + // Production shape: private, owned manager (ownsManager=true). + CursorSendEngine engine = new CursorSendEngine(slot, segSize); + SegmentManager manager = readManager(engine); + long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT); + try { + // Phase 1: let the worker finish the initial spare install so + // the hook below can only fire on the rotation-triggered pass. + SegmentRing ring = readRing(engine); + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (ring.needsHotSpare()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("manager worker never installed the initial hot spare"); + } + Thread.sleep(1); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + + // Phase 2: one frame fills the active segment exactly; the + // second forces rotation onto the spare. needsHotSpare() is + // true again, so the worker's next tick parks in the hook. + Unsafe.getUnsafe().putLong(buf, 0L); + Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen)); + Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen)); + Assert.assertTrue("worker never re-entered a spare-install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Phase 3: owned close with the worker provably mid service + // pass. manager.close()'s 50 ms join times out, the worker is + // not reaped, and close() must retain every worker-reachable + // resource — above all the slot flock. + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse("incomplete owned close must remain observable to the owner", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("owned engine.close() released the slot lock while its manager " + + "worker was still mid service pass — a replacement engine could " + + "acquire the slot and have its segment files unlinked by the " + + "stale worker (the production SF-data-loss hazard)"); + } catch (Exception expected) { + // good — slot retained. + } + + // Phase 4: release the worker (it abandons the spare: the ring + // was deregistered by the close attempt) and retry. The join + // now reaps the worker and the full cleanup must complete. + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + engine.close(); + Assert.assertTrue("retried owned close must report complete cleanup", + engine.isCloseCompleted()); + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after a completed close", probe); + } catch (Exception e) { + throw new AssertionError("retried owned close() did not release the slot lock", e); + } + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT); + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + try { + engine.close(); + } catch (Throwable ignored) { + } + } + }); + } + + /** + * The ownership handoff (owned manager): when close() cannot confirm + * worker quiescence within the bounded join, the terminal cleanup (ring, + * watermark, flock release) transfers to the worker's exit path — the + * worker is provably the last thread able to touch the slot directory. + * Once the parked pass is released the worker must run the cleanup + * itself, WITHOUT any retried {@code close()}: {@code isCloseCompleted()} + * flips true and the slot becomes acquirable again. This is what lets a + * pool recover a retired slot instead of losing its capacity until + * process exit. + */ + @Test(timeout = 30_000L) + public void testOwnedEngineCloseHandsCleanupToWorkerExit() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int payloadLen = 32; + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen); + String slot = tmpDir + "/owned-handoff-slot"; + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + // Production shape: private, owned manager (ownsManager=true). + CursorSendEngine engine = new CursorSendEngine(slot, segSize); + SegmentManager manager = readManager(engine); + long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT); + try { + // Phase 1: wait out the initial spare install so the park hook + // can only fire on the rotation-triggered pass (see + // testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass). + SegmentRing ring = readRing(engine); + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (ring.needsHotSpare()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("manager worker never installed the initial hot spare"); + } + Thread.sleep(1); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + + // Phase 2: fill the active segment and rotate onto the spare; + // the worker's next tick re-enters the install pass and parks. + Unsafe.getUnsafe().putLong(buf, 0L); + Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen)); + Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen)); + Assert.assertTrue("worker never re-entered a spare-install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Phase 3: owned close with the worker provably mid-pass. The + // bounded join times out; cleanup ownership is handed to the + // worker's exit path. Every worker-reachable resource — above + // all the slot flock — must still be retained at this point. + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse("close must stay incomplete while the worker holds the handoff", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("engine.close() released the slot lock while its manager worker " + + "was still mid service pass — the handoff must not weaken the " + + "quiescence gate"); + } catch (Exception expected) { + // good — slot retained while the worker can still touch it. + } + + // Phase 4 — the contract under test: release the worker and do + // NOT retry close(). The worker finishes its pass, exits, and + // runs the deferred cleanup itself, flipping isCloseCompleted + // and releasing the flock with no further caller action. + releaseWorker.countDown(); + long cleanupDeadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!engine.isCloseCompleted()) { + if (System.nanoTime() > cleanupDeadlineNs) { + throw new AssertionError( + "deferred cleanup never ran on manager-worker exit — the slot " + + "would stay retired until process exit"); + } + Thread.sleep(1); + } + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after the worker-exit cleanup", probe); + } catch (Exception e) { + throw new AssertionError("worker-exit cleanup did not release the slot lock", e); + } + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT); + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + try { + engine.close(); + } catch (Throwable ignored) { + } + } + }); + } + + /** + * Registration-failure twin of + * {@link #testOwnedEngineCloseHandsCleanupToWorkerExit}: when + * {@code deferUntilWorkerExit} itself throws (allocation failure while + * building the handoff), close() must NOT mistake the swallowed throw + * for "worker already exited" and run the terminal cleanup inline — the + * worker is provably still mid service pass, so releasing the ring, + * watermark or slot flock here is the original stale-worker UAF/data-loss + * hazard. Every worker-reachable resource must be retained and the close + * must stay incomplete; a retried close() after the worker exits + * converges and releases the slot. + */ + @Test(timeout = 30_000L) + public void testOwnedEngineCloseRetainsSlotWhenHandoffRegistrationFails() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int payloadLen = 32; + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen); + String slot = tmpDir + "/owned-regfail-slot"; + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + // Production shape: private, owned manager (ownsManager=true). + CursorSendEngine engine = new CursorSendEngine(slot, segSize); + SegmentManager manager = readManager(engine); + long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT); + try { + // Phase 1: wait out the initial spare install so the park hook + // can only fire on the rotation-triggered pass (see + // testOwnedEngineCloseRetainsSlotWhileWorkerIsMidServicePass). + SegmentRing ring = readRing(engine); + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (ring.needsHotSpare()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("manager worker never installed the initial hot spare"); + } + Thread.sleep(1); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + + // Phase 2: fill the active segment and rotate onto the spare; + // the worker's next tick re-enters the install pass and parks. + Unsafe.getUnsafe().putLong(buf, 0L); + Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen)); + Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen)); + Assert.assertTrue("worker never re-entered a spare-install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Phase 3: make the handoff registration throw — simulating + // an OutOfMemoryError allocating the cleanup lambda/list — + // with the worker provably still mid service pass. close() + // must retain everything: no inline finishClose, no flock + // release, closeCompleted stays false. + manager.setBeforeExitCleanupRegistrationHook(() -> { + throw new OutOfMemoryError("simulated allocation failure registering exit cleanup"); + }); + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse("close must stay incomplete when handoff registration fails", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("engine.close() released the slot lock after a failed handoff " + + "registration while the manager worker was still mid service " + + "pass — the swallowed throw was mistaken for proof the worker " + + "exited (stale-worker UAF/data-loss hazard)"); + } catch (Exception expected) { + // good — slot retained while the worker can still touch it. + } + + // Phase 4: clear the fault, release the worker (its loop was + // already stopped by the close attempt, so it exits), and + // retry close(). The retry must converge via isWorkerReaped() + // and release the slot. + manager.setBeforeExitCleanupRegistrationHook(null); + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + engine.close(); + Assert.assertTrue("retried close must report complete cleanup", + engine.isCloseCompleted()); + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after the retried close", probe); + } catch (Exception e) { + throw new AssertionError("retried close() did not release the slot lock", e); + } + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT); + manager.setBeforeInstallSyncHook(null); + manager.setBeforeExitCleanupRegistrationHook(null); + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + try { + engine.close(); + } catch (Throwable ignored) { + } + } + }); + } + + /** + * Exactly-once contention on the terminal-cleanup claim + * ({@code terminalCleanupClaimed} CAS): after a timed-out owned close + * handed cleanup to the worker's exit path, a retried {@code close()} + * that races the worker MID-{@code finishClose} must neither re-run the + * terminal cleanup (double munmap / double flock release) nor block on + * the worker, nor publish completion on the worker's behalf. The race + * window is made deterministic by parking the worker inside + * {@code finishClose} (via {@code beforeFlockReleaseHook}) while the + * retried close() converges through {@code isWorkerReaped()} and loses + * the CAS. + */ + @Test(timeout = 30_000L) + public void testTerminalCleanupRunsExactlyOnceWhenRetriedCloseRacesWorkerHandoff() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int payloadLen = 32; + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen); + String slot = tmpDir + "/cas-contention-slot"; + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + CountDownLatch inFinishClose = new CountDownLatch(1); + CountDownLatch releaseFinishClose = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicInteger finishCloseRuns = new AtomicInteger(); + AtomicReference hookErr = new AtomicReference<>(); + // Production shape: private, owned manager (ownsManager=true). + CursorSendEngine engine = new CursorSendEngine(slot, segSize); + SegmentManager manager = readManager(engine); + long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT); + try { + // Phase 1: wait out the initial spare install so the park hook + // can only fire on the rotation-triggered pass. + SegmentRing ring = readRing(engine); + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (ring.needsHotSpare()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("manager worker never installed the initial hot spare"); + } + Thread.sleep(1); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + // Counts terminal-cleanup executions and parks the FIRST one + // (the worker's deferred cleanup) mid-finishClose, before the + // flock release — the exact window a retried close() races. + engine.setBeforeFlockReleaseHook(() -> { + if (finishCloseRuns.incrementAndGet() == 1) { + inFinishClose.countDown(); + try { + if (!releaseFinishClose.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, new AssertionError( + "timed out waiting for test to release finishClose")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + } + }); + + // Phase 2: rotate onto the spare so the worker parks in the + // next install pass. + Unsafe.getUnsafe().putLong(buf, 0L); + Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen)); + Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen)); + Assert.assertTrue("worker never re-entered a spare-install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Phase 3: timed-out close — cleanup ownership transfers to + // the worker's exit path. + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse("close must stay incomplete while the worker holds the handoff", + engine.isCloseCompleted()); + + // Phase 4: release the pass. The worker exits its loop, wins + // the cleanup CAS, enters finishClose and parks in the hook — + // mid-cleanup, flock still held, completion unpublished. + releaseWorker.countDown(); + Assert.assertTrue("worker never entered the deferred finishClose", + inFinishClose.await(10, TimeUnit.SECONDS)); + Assert.assertEquals(1, finishCloseRuns.get()); + Assert.assertFalse("completion must not be observable mid-finishClose", + engine.isCloseCompleted()); + + // Phase 5 — the contention under test: a retried close() while + // the worker is parked INSIDE finishClose. The worker loop has + // already exited (workerLoopExited=true precedes the deferred + // cleanups), so the short bounded join reaps the manager state + // and close() converges to the CAS — which it must LOSE, + // returning promptly without touching ring/watermark/flock. + engine.close(); + Assert.assertEquals( + "retried close() re-ran the terminal cleanup while the worker's " + + "deferred cleanup was mid-flight — ring/watermark/flock would " + + "be double-released", + 1, finishCloseRuns.get()); + Assert.assertFalse("retried close() must not publish completion on the worker's behalf", + engine.isCloseCompleted()); + try { + SlotLock probe = SlotLock.acquire(slot); + probe.close(); + Assert.fail("slot lock observable as released while the worker was still " + + "parked before its flock release"); + } catch (Exception expected) { + // good — flock still held by the parked cleanup. + } + + // Phase 6: let the worker finish. Completion publishes, the + // slot becomes acquirable, and the cleanup count stays at 1. + releaseFinishClose.countDown(); + long cleanupDeadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!engine.isCloseCompleted()) { + if (System.nanoTime() > cleanupDeadlineNs) { + throw new AssertionError("deferred cleanup never completed after release"); + } + Thread.sleep(1); + } + Assert.assertEquals(1, finishCloseRuns.get()); + try (SlotLock probe = SlotLock.acquire(slot)) { + Assert.assertNotNull("slot must be acquirable after the worker-exit cleanup", probe); + } + // A final close() takes the fast no-op path. + engine.close(); + Assert.assertEquals("post-completion close() must be a no-op", + 1, finishCloseRuns.get()); + if (hookErr.get() != null) { + throw new AssertionError("hook failed", hookErr.get()); + } + } finally { + Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT); + manager.setBeforeInstallSyncHook(null); + engine.setBeforeFlockReleaseHook(null); + releaseWorker.countDown(); + releaseFinishClose.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + try { + engine.close(); + } catch (Throwable ignored) { + } + } + }); + } + + /** + * Memory-mode twin of {@link #testOwnedEngineCloseHandsCleanupToWorkerExit}: + * {@code sfDir == null}, so there is no slot lock, no watermark and no + * segment files — but the ring's malloc'd native segments are still + * worker-reachable, so the timed-out close must take the same handoff + * path with every SF-only resource null. Pins that (a) the handoff + * branch tolerates null slotLock/watermark/sfDir without NPE, (b) the + * close stays incomplete while the worker can still touch the ring, and + * (c) the worker-exit cleanup completes the close and frees the ring's + * native memory (assertMemoryLeak is the leak oracle here). + */ + @Test(timeout = 30_000L) + public void testMemoryModeOwnedCloseHandsCleanupToWorkerExit() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int payloadLen = 32; + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + payloadLen); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + // Memory mode: null sfDir, private owned manager — the exact + // shape non-SF async ingest uses. + CursorSendEngine engine = new CursorSendEngine(null, segSize); + SegmentManager manager = readManager(engine); + long buf = Unsafe.malloc(payloadLen, MemoryTag.NATIVE_DEFAULT); + try { + // Phase 1: wait out the initial spare install so the park hook + // can only fire on the rotation-triggered pass. + SegmentRing ring = readRing(engine); + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (ring.needsHotSpare()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("manager worker never installed the initial hot spare"); + } + Thread.sleep(1); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + + // Phase 2: rotate onto the spare; the worker's next tick + // re-enters the (in-memory) install pass and parks. + Unsafe.getUnsafe().putLong(buf, 0L); + Assert.assertEquals(0L, engine.appendBlocking(buf, payloadLen)); + Assert.assertEquals(1L, engine.appendBlocking(buf, payloadLen)); + Assert.assertTrue("worker never re-entered a spare-install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Phase 3: timed-out memory-mode close. The worker can still + // touch the ring's native memory, so the close must hand off + // and stay incomplete — releasing the ring here would be a + // use-after-free on the worker's install path. + manager.setWorkerJoinTimeoutMillis(50L); + engine.close(); + Assert.assertFalse( + "memory-mode close must stay incomplete while the worker is mid service pass", + engine.isCloseCompleted()); + + // Phase 4: release the worker; its exit path must run the + // deferred cleanup (null slotLock/watermark/sfDir) and flip + // completion with no further caller action. + releaseWorker.countDown(); + long cleanupDeadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!engine.isCloseCompleted()) { + if (System.nanoTime() > cleanupDeadlineNs) { + throw new AssertionError( + "deferred memory-mode cleanup never ran on manager-worker exit — " + + "the ring's native segments would leak for the process lifetime"); + } + Thread.sleep(1); + } + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + Unsafe.free(buf, payloadLen, MemoryTag.NATIVE_DEFAULT); + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + try { + engine.close(); + } catch (Throwable ignored) { + } + } + }); + } + + /** + * An engine that owns its manager must use the whole-manager stop/join as + * its only quiescence barrier. Calling the per-ring barrier first would + * give a stuck worker two independent timeout budgets. + */ + @Test(timeout = 30_000L) + public void testOwnedManagerCloseSkipsPerRingQuiescenceWait() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String slot = tmpDir + "/owned-slot"; + CursorSendEngine engine = new CursorSendEngine(slot, 4L * 1024 * 1024); + SegmentManager manager = readManager(engine); + AtomicBoolean perRingAwaited = new AtomicBoolean(); + try { + manager.setBeforeRingQuiescenceAwaitHook(() -> perRingAwaited.set(true)); + engine.close(); + Assert.assertTrue("owned engine close did not complete", engine.isCloseCompleted()); + Assert.assertFalse("owned engine close spent a separate per-ring wait budget", + perRingAwaited.get()); + } finally { + manager.setBeforeRingQuiescenceAwaitHook(null); + engine.close(); + } + }); + } + + /** + * Plain-positive path: after a normal close (worker quiesces promptly), + * a second engine must be able to acquire and use the same slot. + */ + @Test(timeout = 30_000L) + public void testSameSlotReacquirableAfterNormalClose() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String slot = tmpDir + "/slot"; + CursorSendEngine first = new CursorSendEngine(slot, 4L * 1024 * 1024); + first.close(); + CursorSendEngine second = new CursorSendEngine(slot, 4L * 1024 * 1024); + try { + Assert.assertFalse("fully-drained close must leave no segments to recover", + second.wasRecoveredFromDisk()); + } finally { + second.close(); + } + }); + } + + private static SegmentManager readManager(CursorSendEngine engine) throws Exception { + Field field = CursorSendEngine.class.getDeclaredField("manager"); + field.setAccessible(true); + return (SegmentManager) field.get(engine); + } + + private static SegmentRing readRing(CursorSendEngine engine) throws Exception { + Field field = CursorSendEngine.class.getDeclaredField("ring"); + field.setAccessible(true); + return (SegmentRing) field.get(engine); + } + + private static void rmDirRecursive(String dir) { + if (!Files.exists(dir)) return; + long find = Files.findFirst(dir); + if (find <= 0) return; + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + String child = dir + "/" + name; + if (!Files.remove(child)) { + rmDirRecursive(child); + Files.remove(child); + } + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineTest.java index f4de1ff2..c11fe1fe 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorSendEngineTest.java @@ -197,6 +197,25 @@ public void testCloseIsIdempotent() throws Exception { }); } + @Test + public void testCallbackCreationFailurePrecedesOwnedManagerResources() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CursorSendEngine.setBeforeDeferredCloseCreationHook(() -> { + throw new OutOfMemoryError("simulated bound callback allocation failure"); + }); + try { + try { + new CursorSendEngine(tmpDir, 4096); + fail("expected callback allocation failure"); + } catch (OutOfMemoryError expected) { + assertEquals("simulated bound callback allocation failure", expected.getMessage()); + } + } finally { + CursorSendEngine.setBeforeDeferredCloseCreationHook(null); + } + }); + } + @Test public void testConstructorFailureAfterOwnedManagerStartCleansResources() throws Exception { TestUtils.assertMemoryLeak(() -> { diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoopBlockedSendCloseTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoopBlockedSendCloseTest.java new file mode 100644 index 00000000..0a53e28e --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/CursorWebSocketSendLoopBlockedSendCloseTest.java @@ -0,0 +1,256 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.DefaultHttpClientConfiguration; +import io.questdb.client.cutlass.http.client.WebSocketClient; +import io.questdb.client.cutlass.line.LineSenderException; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop; +import io.questdb.client.network.PlainSocketFactory; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import io.questdb.client.test.tools.TestUtils; +import org.junit.Assert; +import org.junit.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +public class CursorWebSocketSendLoopBlockedSendCloseTest { + + @Test(timeout = 30_000L) + public void testCloseBreaksBlockedSendBeforeJoiningWorker() throws Exception { + TestUtils.assertMemoryLeak(() -> { + BlockingSendClient client = new BlockingSendClient(true); + CursorSendEngine engine = new CursorSendEngine(null, 64 * 1024); + CursorWebSocketSendLoop loop = new CursorWebSocketSendLoop( + client, + engine, + 0L, + CursorWebSocketSendLoop.DEFAULT_PARK_NANOS, + null, + 100L, + 1_000L, + 5_000L, + false + ); + long payload = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT); + Thread closer = null; + AtomicReference closeFailure = new AtomicReference<>(); + try { + Unsafe.getUnsafe().putLong(payload, 0x0102030405060708L); + Unsafe.getUnsafe().putLong(payload + 8, 0x1112131415161718L); + Assert.assertEquals(0L, engine.appendBlocking(payload, 16)); + loop.start(); + Assert.assertTrue("I/O worker never entered the blocking send", + client.sendEntered.await(5, TimeUnit.SECONDS)); + + closer = new Thread(() -> { + try { + loop.close(); + } catch (Throwable t) { + closeFailure.set(t); + } + }, "blocked-send-closer"); + closer.start(); + + Assert.assertTrue("close did not break the traffic path before joining", + client.trafficClosed.await(5, TimeUnit.SECONDS)); + Assert.assertEquals("traffic path must close exactly once", 1, client.trafficCloseCount.get()); + Assert.assertEquals("the closer must break traffic, not the I/O worker", + closer, client.trafficCloseThread.get()); + Assert.assertTrue("blocked send did not observe traffic-path closure", + client.sendExited.await(5, TimeUnit.SECONDS)); + + closer.join(TimeUnit.SECONDS.toMillis(5)); + Assert.assertFalse("close did not join the I/O worker", closer.isAlive()); + Assert.assertNull("close failed", closeFailure.get()); + Assert.assertNull("ordinary close must not manufacture a terminal error", loop.getTerminalError()); + + Thread ioThread = client.sendThread.get(); + Assert.assertNotNull(ioThread); + Assert.assertFalse("I/O worker must be dead when close returns", ioThread.isAlive()); + Assert.assertEquals("full client cleanup must run exactly once", 1, client.cleanupCount.get()); + Assert.assertEquals("the I/O worker must own cleanup before publishing exit", + ioThread, client.cleanupThread.get()); + Assert.assertFalse("close must not manufacture caller interruption", + closer.isInterrupted()); + } finally { + client.releaseSend.countDown(); + if (closer != null) { + closer.join(TimeUnit.SECONDS.toMillis(5)); + } + loop.close(); + engine.close(); + client.close(); + Unsafe.free(payload, 16, MemoryTag.NATIVE_DEFAULT); + } + }); + } + + @Test(timeout = 30_000L) + public void testUnsupportedCustomTransportFailsWithoutDestroyingWorkerResources() throws Exception { + TestUtils.assertMemoryLeak(() -> { + BlockingSendClient client = new BlockingSendClient(false); + CursorSendEngine engine = new CursorSendEngine(null, 64 * 1024); + CursorWebSocketSendLoop loop = new CursorWebSocketSendLoop( + client, + engine, + 0L, + CursorWebSocketSendLoop.DEFAULT_PARK_NANOS, + null, + 100L, + 1_000L, + 5_000L, + false + ); + long payload = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT); + Thread closer = null; + AtomicReference closeFailure = new AtomicReference<>(); + CountDownLatch closeEntered = new CountDownLatch(1); + try { + Unsafe.getUnsafe().putLong(payload, 0x0102030405060708L); + Unsafe.getUnsafe().putLong(payload + 8, 0x1112131415161718L); + Assert.assertEquals(0L, engine.appendBlocking(payload, 16)); + loop.start(); + Assert.assertTrue("I/O worker never entered the blocking send", + client.sendEntered.await(5, TimeUnit.SECONDS)); + + closer = new Thread(() -> { + closeEntered.countDown(); + try { + loop.close(); + } catch (Throwable t) { + closeFailure.set(t); + } + }, "unsupported-transport-closer"); + closer.start(); + + Assert.assertTrue("close did not start", closeEntered.await(5, TimeUnit.SECONDS)); + closer.join(TimeUnit.SECONDS.toMillis(5)); + Assert.assertFalse("unsupported transport made close join indefinitely", closer.isAlive()); + Assert.assertTrue(closeFailure.get() instanceof LineSenderException); + Assert.assertTrue(closeFailure.get().getCause() instanceof UnsupportedOperationException); + Assert.assertEquals("unsupported cancellation must not release the blocked send", + 1L, client.sendExited.getCount()); + Assert.assertEquals("unsupported cancellation must not perform full cleanup", 0, client.cleanupCount.get()); + Assert.assertTrue("worker must retain resource ownership", client.sendThread.get().isAlive()); + + client.releaseSend.countDown(); + Assert.assertTrue("released send did not exit", client.sendExited.await(5, TimeUnit.SECONDS)); + Assert.assertTrue("worker did not complete delegated cleanup", client.cleanupDone.await(5, TimeUnit.SECONDS)); + client.sendThread.get().join(TimeUnit.SECONDS.toMillis(5)); + Assert.assertFalse("worker lingered after the custom transport was released", client.sendThread.get().isAlive()); + Assert.assertEquals(1, client.cleanupCount.get()); + Assert.assertNull("ordinary worker exit must remain non-terminal", loop.getTerminalError()); + } finally { + client.releaseSend.countDown(); + if (closer != null) { + closer.join(TimeUnit.SECONDS.toMillis(5)); + } + Thread ioThread = client.sendThread.get(); + if (ioThread != null) { + ioThread.join(TimeUnit.SECONDS.toMillis(5)); + } + loop.close(); + engine.close(); + client.close(); + Unsafe.free(payload, 16, MemoryTag.NATIVE_DEFAULT); + } + }); + } + + private static final class BlockingSendClient extends WebSocketClient { + private final AtomicBoolean cleanupClaimed = new AtomicBoolean(); + private final AtomicInteger cleanupCount = new AtomicInteger(); + private final CountDownLatch cleanupDone = new CountDownLatch(1); + private final AtomicReference cleanupThread = new AtomicReference<>(); + private final boolean trafficShutdownSupported; + private final CountDownLatch releaseSend = new CountDownLatch(1); + private final CountDownLatch sendEntered = new CountDownLatch(1); + private final CountDownLatch sendExited = new CountDownLatch(1); + private final AtomicReference sendThread = new AtomicReference<>(); + private final AtomicInteger trafficCloseCount = new AtomicInteger(); + private final AtomicReference trafficCloseThread = new AtomicReference<>(); + private final CountDownLatch trafficClosed = new CountDownLatch(1); + + private BlockingSendClient(boolean trafficShutdownSupported) { + super(DefaultHttpClientConfiguration.INSTANCE, PlainSocketFactory.INSTANCE); + this.trafficShutdownSupported = trafficShutdownSupported; + } + + @Override + public void close() { + if (cleanupClaimed.compareAndSet(false, true)) { + cleanupCount.incrementAndGet(); + cleanupThread.set(Thread.currentThread()); + cleanupDone.countDown(); + } + super.close(); + } + + @Override + public void closeTraffic() { + if (!trafficShutdownSupported) { + throw new UnsupportedOperationException("custom transport has no safe cancellation capability"); + } + trafficCloseThread.compareAndSet(null, Thread.currentThread()); + trafficCloseCount.incrementAndGet(); + trafficClosed.countDown(); + releaseSend.countDown(); + } + + @Override + public void sendBinary(long dataPtr, int length, int timeout) { + sendThread.set(Thread.currentThread()); + sendEntered.countDown(); + try { + if (!releaseSend.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("traffic path was not closed"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError("I/O worker was interrupted instead of traffic being closed", e); + } finally { + sendExited.countDown(); + } + throw new LineSenderException("traffic path closed"); + } + + @Override + protected void ioWait(int timeout, int op) { + throw new UnsupportedOperationException("stub: no socket"); + } + + @Override + protected void setupIoWait() { + // no-op + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/EngineClosePublishAfterFlockReleaseTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/EngineClosePublishAfterFlockReleaseTest.java new file mode 100644 index 00000000..c142dd6a --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/EngineClosePublishAfterFlockReleaseTest.java @@ -0,0 +1,242 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock; +import io.questdb.client.std.Files; +import io.questdb.client.test.tools.TestUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.nio.file.Paths; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Regression test for the completion/release ordering in + * {@code CursorSendEngine.finishClose()}: {@code closeCompleted} must be + * published strictly AFTER the slot flock release is confirmed, never + * before. + * + *

    The bug being pinned: {@code closeCompleted = true} used to be written + * before {@code slotLock} was closed. A pool thread could observe completion + * through {@code QwpWebSocketSender.isSlotLockReleased()}, free the slot + * index in {@code SenderPool.reprobeRetiredSlots()}, and admit a replacement + * sender whose {@code SlotLock.acquire} then collided with the still-open + * flock fd — a spurious "sf slot already in use" construction failure naming + * the process's own pid as the holder. + * + *

    The window between the publish and the {@code Files.close(fd)} is + * microseconds wide in the wild; {@code setBeforeFlockReleaseHook} makes it + * deterministic. The closing thread is parked between terminal cleanup and + * the flock release, and the test asserts from outside the window's two + * halves of the contract: + *

    + */ +public class EngineClosePublishAfterFlockReleaseTest { + + private String sfDir; + + @Before + public void setUp() { + sfDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-engine-close-publish-order-" + System.nanoTime()).toString(); + } + + @After + public void tearDown() { + if (sfDir == null) return; + long find = Files.findFirst(sfDir); + if (find > 0) { + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + Files.remove(sfDir + "/" + name); + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } + Files.remove(sfDir); + } + + @Test(timeout = 30_000L) + public void testCloseCompletedPublishedOnlyAfterConfirmedFlockRelease() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CursorSendEngine engine = new CursorSendEngine(sfDir, 4L * 1024 * 1024); + CountDownLatch inWindow = new CountDownLatch(1); + CountDownLatch proceed = new CountDownLatch(1); + engine.setBeforeFlockReleaseHook(() -> { + inWindow.countDown(); + try { + // Bounded so a failed assertion on the main thread can + // never wedge the closer past the test timeout. + proceed.await(20, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + + AtomicReference closerError = new AtomicReference<>(); + Thread closer = new Thread(() -> { + try { + engine.close(); + } catch (Throwable t) { + closerError.set(t); + } + }, "engine-closer"); + closer.start(); + + try { + assertTrue("closer thread never reached the cleanup/release window", + inWindow.await(10, TimeUnit.SECONDS)); + + // Inside the window: terminal cleanup has run, the flock has + // NOT been released. Completion must not be observable yet — + // this is the exact read a pool thread performs before + // freeing the slot index. + assertFalse("closeCompleted was published before the flock release; " + + "a pool observing this would free the slot index and a " + + "replacement sender would collide with the still-held flock", + engine.isCloseCompleted()); + + // Prove the window is real: the flock is genuinely still + // held, so acquisition by a "replacement engine" fails. + try { + SlotLock probe = SlotLock.acquire(sfDir); + probe.close(); + fail("scaffolding error: expected the slot flock to still be " + + "held inside the pre-release window, but a fresh " + + "SlotLock.acquire succeeded"); + } catch (IllegalStateException expected) { + // good — slot is locked, which is why completion must + // not have been published yet. + } + } finally { + proceed.countDown(); + closer.join(10_000L); + } + assertFalse("closer thread did not finish", closer.isAlive()); + assertNull("engine.close() threw", closerError.get()); + + // After the window: the release is confirmed, so completion must + // now be latched, and completion must still imply reusability. + assertTrue("closeCompleted must latch once the flock release is confirmed", + engine.isCloseCompleted()); + try (SlotLock ignored = SlotLock.acquire(sfDir)) { + // good — completion implies the slot dir is acquirable. + } catch (IllegalStateException stillHeld) { + fail("closeCompleted reported true but the slot flock is still held: " + + stillHeld.getMessage()); + } + }); + } + + /** + * The error half of the publish-after-release contract: when + * {@code SlotLock.release()} reports failure (the OS refused the explicit + * unlock, so the flock may still be held), {@code closeCompleted} must + * stay false until a later close confirms release. A pool observing + * {@code isCloseCompleted() == false} keeps the slot retired while the + * flock is genuinely held, then recovers it once the retry completes. + *

    + * The lock's fd is swapped to a known-bad descriptor for a deterministic + * native unlock failure, then restored before retrying through the engine's + * public close API. + */ + @Test(timeout = 30_000L) + public void testUnconfirmedFlockReleaseKeepsCloseIncompleteUntilRetry() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CursorSendEngine engine = new CursorSendEngine(sfDir, 4L * 1024 * 1024); + Field slotLockField = CursorSendEngine.class.getDeclaredField("slotLock"); + slotLockField.setAccessible(true); + SlotLock slotLock = (SlotLock) slotLockField.get(engine); + assertNotNull("disk-mode engine must hold a slot lock", slotLock); + Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int realFd = fdField.getInt(slotLock); + assertTrue("precondition: live flock fd", realFd >= 0); + try { + // A non-negative fd no process has open: close(2) fails EBADF, + // so finishClose's release() confirmation fails. + fdField.setInt(slotLock, 1_000_000_000); + engine.close(); + assertFalse( + "closeCompleted was published despite an unconfirmed flock release; " + + "a pool observing this would free the slot index while the " + + "flock fd is still open", + engine.isCloseCompleted()); + // The REAL flock is still held — the incomplete report is true. + try { + SlotLock probe = SlotLock.acquire(sfDir); + probe.close(); + fail("slot must not be acquirable while the original flock fd is still open"); + } catch (IllegalStateException expected) { + // good — incomplete close really means "still locked". + } + + // Remove the injected fault. The retry must only re-attempt + // the retained fd release: ring/watermark cleanup stays + // exactly-once, while completion and slot reusability recover. + fdField.setInt(slotLock, realFd); + engine.close(); + assertTrue("retried close() must complete after the flock release succeeds", + engine.isCloseCompleted()); + try (SlotLock ignored = SlotLock.acquire(sfDir)) { + // good — eventual completion implies reusable capacity. + } + } finally { + // If an assertion failed before the successful retry, restore + // and release the real fd so the test never leaks a flock. + if (!engine.isCloseCompleted()) { + fdField.setInt(slotLock, realFd); + assertTrue("restored fd must release cleanly", slotLock.release()); + } + } + }); + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/FlockReleaseRetryDriverTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/FlockReleaseRetryDriverTest.java new file mode 100644 index 00000000..0037437f --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/FlockReleaseRetryDriverTest.java @@ -0,0 +1,508 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock; +import io.questdb.client.std.Files; +import io.questdb.client.test.tools.TestUtils; +import org.junit.After; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +public class FlockReleaseRetryDriverTest { + + private final List sfDirs = new ArrayList<>(); + + @After + public void tearDown() { + CursorSendEngine.setAfterFlockReleaseRetryFailureHook(null); + CursorSendEngine.setFlockReleaseRetryParkOverride(null); + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + for (String sfDir : sfDirs) { + removeDir(sfDir); + } + } + + /** + * Driver-start failure must not strand retired capacity until process + * exit. {@code Sender.close()} is a one-shot no-op by contract, so the + * only recovery surface a pool has is its retired-slot probe + * ({@code isSlotLockReleased()}, called from the housekeeper tick and + * capacity-starved borrows) — that probe must re-arm the shared retry + * driver once thread creation works again. + */ + @Test(timeout = 30_000L) + public void testDriverStartFailureRecoversViaPoolProbe() throws Exception { + TestUtils.assertMemoryLeak(() -> { + AtomicInteger starts = new AtomicInteger(); + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> new Thread(task) { + @Override + public synchronized void start() { + starts.incrementAndGet(); + throw new IllegalStateException("injected start failure"); + } + }); + + CursorSendEngine engine = new CursorSendEngine( + newSfDir("probe-rearm"), 4L * 1024 * 1024); + SlotLock slotLock = slotLock(engine); + int realFd = fd(slotLock); + QwpWebSocketSender sender = QwpWebSocketSender.createForTesting("localhost", 1); + sender.setCursorEngine(engine, true); + AtomicReference rearmedDriver = new AtomicReference<>(); + boolean recovered = false; + try { + setFd(slotLock, 1_000_000_000); + sender.close(); + assertEquals("retry driver start must be attempted once", 1, starts.get()); + assertFalse("failed unlock must remain unpublished", engine.isCloseCompleted()); + + // Sender.close() is idempotent: repeat calls never reach the + // engine again, so they cannot restart the failed driver. + sender.close(); + assertEquals("no-op close must not retry the driver start", 1, starts.get()); + + // While the fault persists, each pool probe re-attempts the + // re-arm (and fails again) without publishing a release. + assertFalse("failed unlock must keep the slot reported as held", + sender.isSlotLockReleased()); + assertTrue("pool probe must re-attempt the driver start", + starts.get() >= 2); + + // The transient condition clears: thread creation works again + // (the failed start drained the queue, so the factory swap is + // legal) and the flock fd is back. + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> { + Thread thread = new Thread(task, "test-rearmed-flock-release-retry"); + rearmedDriver.set(thread); + return thread; + }); + setFd(slotLock, realFd); + CountDownLatch released = new CountDownLatch(1); + engine.setSlotLockReleaseListener(released::countDown); + + // Production recovery surface: the pool re-probes the retired + // slot through isSlotLockReleased(). + sender.isSlotLockReleased(); + assertTrue("pool probe must re-arm the flock-release retry after " + + "a driver start failure", + released.await(10, TimeUnit.SECONDS)); + assertTrue(engine.isCloseCompleted()); + assertTrue("probe must expose the recovered release", + sender.isSlotLockReleased()); + recovered = true; + } finally { + Thread driver = rearmedDriver.get(); + if (driver != null) { + driver.join(10_000L); + } + if (!recovered) { + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + setFd(slotLock, realFd); + if (!slotLock.release()) { + fail("restored flock fd did not release"); + } + } + } + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + }); + } + + @Test(timeout = 30_000L) + public void testPersistentFailuresShareOneRetryThread() throws Exception { + TestUtils.assertMemoryLeak(() -> { + final int engineCount = 32; + AtomicInteger retryFailures = new AtomicInteger(); + AtomicInteger threadsCreated = new AtomicInteger(); + AtomicReference retryThreadRef = new AtomicReference<>(); + CountDownLatch retryFailuresObserved = new CountDownLatch(engineCount * 2); + CountDownLatch retryThreadStarted = new CountDownLatch(1); + CountDownLatch runRetryDriver = new CountDownLatch(1); + CursorSendEngine.setAfterFlockReleaseRetryFailureHook(() -> { + retryFailures.incrementAndGet(); + retryFailuresObserved.countDown(); + }); + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> { + threadsCreated.incrementAndGet(); + Thread thread = new Thread(() -> { + retryThreadStarted.countDown(); + try { + runRetryDriver.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + task.run(); + }, "test-shared-flock-release-retry"); + retryThreadRef.set(thread); + return thread; + }); + + List engines = new ArrayList<>(); + List slotLocks = new ArrayList<>(); + List realFds = new ArrayList<>(); + boolean fdsRestored = false; + try { + for (int i = 0; i < engineCount; i++) { + String sfDir = newSfDir("persistent-" + i); + CursorSendEngine engine = new CursorSendEngine(sfDir, 4L * 1024 * 1024); + SlotLock slotLock = slotLock(engine); + int realFd = fd(slotLock); + engines.add(engine); + slotLocks.add(slotLock); + realFds.add(realFd); + setFd(slotLock, 1_000_000_000); + engine.close(); + assertFalse("injected unlock failure must keep close incomplete", + engine.isCloseCompleted()); + if (i == 0) { + assertTrue("retry thread was not started", + retryThreadStarted.await(10, TimeUnit.SECONDS)); + } + } + + assertEquals("persistent failures must share one retry thread", + 1, threadsCreated.get()); + runRetryDriver.countDown(); + assertTrue("driver did not perform two failed rounds", + retryFailuresObserved.await(10, TimeUnit.SECONDS)); + assertTrue("driver did not retain persistent failures", + retryFailures.get() >= engineCount * 2); + for (CursorSendEngine engine : engines) { + assertFalse("failed releases must remain unpublished", + engine.isCloseCompleted()); + } + + restoreFds(slotLocks, realFds); + fdsRestored = true; + Thread retryThread = retryThreadRef.get(); + retryThread.join(10_000L); + assertFalse("shared retry thread retained lifecycle resources after drain", + retryThread.isAlive()); + for (CursorSendEngine engine : engines) { + assertTrue("driver must release every restored flock", + engine.isCloseCompleted()); + } + assertEquals("retries must not create another thread", + 1, threadsCreated.get()); + } finally { + if (!fdsRestored) { + restoreFds(slotLocks, realFds); + } + runRetryDriver.countDown(); + Thread retryThread = retryThreadRef.get(); + if (retryThread != null) { + retryThread.join(10_000L); + } + CursorSendEngine.setAfterFlockReleaseRetryFailureHook(null); + } + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + }); + } + + /** + * Schedule inspection for the shared driver's retry cadence: the + * inter-round park must grow exponentially from 100 ms and cap at 5 s, + * so a persistent unlock failure does not burn 10 syscalls per second + * per engine forever. The park override replaces the real park, so the + * test coordinates rounds without wall-clock waits. + */ + @Test(timeout = 30_000L) + public void testRetryBackoffDoublesToCap() throws Exception { + TestUtils.assertMemoryLeak(() -> { + List parks = new ArrayList<>(); + Semaphore parked = new Semaphore(0); + Semaphore proceed = new Semaphore(0); + AtomicReference driverRef = new AtomicReference<>(); + CursorSendEngine.setFlockReleaseRetryParkOverride(nanos -> { + parks.add(nanos); + parked.release(); + proceed.acquireUninterruptibly(); + }); + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> { + Thread thread = new Thread(task, "test-backoff-flock-release-retry"); + driverRef.set(thread); + return thread; + }); + + CursorSendEngine engine = new CursorSendEngine( + newSfDir("backoff-cap"), 4L * 1024 * 1024); + SlotLock slotLock = slotLock(engine); + int realFd = fd(slotLock); + boolean fdRestored = false; + try { + setFd(slotLock, 1_000_000_000); + engine.close(); + assertFalse("injected unlock failure must keep close incomplete", + engine.isCloseCompleted()); + + // Eight failed rounds: enough to observe the full ramp and + // two capped parks. + for (int round = 1; round <= 8; round++) { + assertTrue("driver did not park after failed round " + round, + parked.tryAcquire(10, TimeUnit.SECONDS)); + if (round < 8) { + proceed.release(); + } + } + + setFd(slotLock, realFd); + fdRestored = true; + proceed.release(); + Thread driver = driverRef.get(); + driver.join(10_000L); + assertFalse("driver did not drain after the release succeeded", + driver.isAlive()); + assertTrue("restored flock must be released", engine.isCloseCompleted()); + + List expected = new ArrayList<>(); + expected.add(100_000_000L); + expected.add(200_000_000L); + expected.add(400_000_000L); + expected.add(800_000_000L); + expected.add(1_600_000_000L); + expected.add(3_200_000_000L); + expected.add(5_000_000_000L); + expected.add(5_000_000_000L); + assertEquals("retry parks must double from 100ms and cap at 5s", + expected, parks); + } finally { + if (!fdRestored) { + setFd(slotLock, realFd); + } + proceed.release(1_000); + Thread driver = driverRef.get(); + if (driver != null) { + driver.join(10_000L); + } + } + CursorSendEngine.setFlockReleaseRetryParkOverride(null); + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + }); + } + + /** + * A successful release in a round is progress: the driver must reset its + * backoff to the base so the remaining engines are retried promptly + * while the failure condition is clearing. + */ + @Test(timeout = 30_000L) + public void testRetryBackoffResetsOnProgress() throws Exception { + TestUtils.assertMemoryLeak(() -> { + List parks = new ArrayList<>(); + Semaphore parked = new Semaphore(0); + Semaphore proceed = new Semaphore(0); + AtomicReference driverRef = new AtomicReference<>(); + CursorSendEngine.setFlockReleaseRetryParkOverride(nanos -> { + parks.add(nanos); + parked.release(); + proceed.acquireUninterruptibly(); + }); + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> { + Thread thread = new Thread(task, "test-reset-flock-release-retry"); + driverRef.set(thread); + return thread; + }); + + CursorSendEngine engineA = new CursorSendEngine( + newSfDir("backoff-reset-a"), 4L * 1024 * 1024); + CursorSendEngine engineB = new CursorSendEngine( + newSfDir("backoff-reset-b"), 4L * 1024 * 1024); + SlotLock slotLockA = slotLock(engineA); + SlotLock slotLockB = slotLock(engineB); + int realFdA = fd(slotLockA); + int realFdB = fd(slotLockB); + boolean fdARestored = false; + boolean fdBRestored = false; + try { + setFd(slotLockA, 1_000_000_000); + setFd(slotLockB, 1_000_000_001); + engineA.close(); + engineB.close(); + + // Three failed rounds ramp the backoff to 400ms. + for (int round = 1; round <= 3; round++) { + assertTrue("driver did not park after failed round " + round, + parked.tryAcquire(10, TimeUnit.SECONDS)); + if (round < 3) { + proceed.release(); + } + } + + // Engine A recovers; round 4 has one success and one failure, + // so its park must be back at the 100ms base. + setFd(slotLockA, realFdA); + fdARestored = true; + proceed.release(); + assertTrue("driver did not park after the mixed round", + parked.tryAcquire(10, TimeUnit.SECONDS)); + assertTrue("recovered engine must publish completion", + engineA.isCloseCompleted()); + + setFd(slotLockB, realFdB); + fdBRestored = true; + proceed.release(); + Thread driver = driverRef.get(); + driver.join(10_000L); + assertFalse("driver did not drain after both releases succeeded", + driver.isAlive()); + assertTrue(engineB.isCloseCompleted()); + + List expected = new ArrayList<>(); + expected.add(100_000_000L); + expected.add(200_000_000L); + expected.add(400_000_000L); + expected.add(100_000_000L); + assertEquals("a successful release must reset the backoff to base", + expected, parks); + } finally { + if (!fdARestored) { + setFd(slotLockA, realFdA); + } + if (!fdBRestored) { + setFd(slotLockB, realFdB); + } + proceed.release(1_000); + Thread driver = driverRef.get(); + if (driver != null) { + driver.join(10_000L); + } + } + CursorSendEngine.setFlockReleaseRetryParkOverride(null); + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + }); + } + + @Test(timeout = 30_000L) + public void testRetryThreadStartFailureLeavesExplicitCloseRetryable() throws Exception { + TestUtils.assertMemoryLeak(() -> { + AtomicInteger starts = new AtomicInteger(); + CursorSendEngine.setFlockReleaseRetryThreadFactory(task -> new Thread(task) { + @Override + public synchronized void start() { + starts.incrementAndGet(); + throw new IllegalStateException("injected start failure"); + } + }); + + CursorSendEngine engine = new CursorSendEngine( + newSfDir("start-failure"), 4L * 1024 * 1024); + SlotLock slotLock = slotLock(engine); + int realFd = fd(slotLock); + try { + setFd(slotLock, 1_000_000_000); + engine.close(); + assertEquals("retry driver start must be attempted once", 1, starts.get()); + assertFalse("failed unlock must remain unpublished", engine.isCloseCompleted()); + + // This also proves the failed driver cleared its queue: the + // setter rejects replacement while any engine remains queued. + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + setFd(slotLock, realFd); + engine.close(); + assertTrue("explicit close must recover after retry-thread start failure", + engine.isCloseCompleted()); + } finally { + if (!engine.isCloseCompleted()) { + CursorSendEngine.setFlockReleaseRetryThreadFactory(null); + setFd(slotLock, realFd); + if (!slotLock.release()) { + fail("restored flock fd did not release"); + } + } + } + }); + } + + private static int fd(SlotLock slotLock) throws Exception { + Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + return fdField.getInt(slotLock); + } + + private static void removeDir(String sfDir) { + long find = Files.findFirst(sfDir); + if (find > 0) { + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + Files.remove(sfDir + "/" + name); + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } + Files.remove(sfDir); + } + + private static void restoreFds(List slotLocks, List realFds) throws Exception { + for (int i = 0; i < slotLocks.size(); i++) { + SlotLock slotLock = slotLocks.get(i); + synchronized (slotLock) { + setFd(slotLock, realFds.get(i)); + } + } + } + + private static void setFd(SlotLock slotLock, int fd) throws Exception { + Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + fdField.setInt(slotLock, fd); + } + + private static SlotLock slotLock(CursorSendEngine engine) throws Exception { + Field slotLockField = CursorSendEngine.class.getDeclaredField("slotLock"); + slotLockField.setAccessible(true); + return (SlotLock) slotLockField.get(engine); + } + + private String newSfDir(String suffix) { + String sfDir = Paths.get( + System.getProperty("java.io.tmpdir"), + "qdb-flock-release-retry-" + suffix + "-" + System.nanoTime() + ).toString(); + sfDirs.add(sfDir); + return sfDir; + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerCloseRaceTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerCloseRaceTest.java index 3d7c6a7c..cee8e41d 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerCloseRaceTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerCloseRaceTest.java @@ -204,6 +204,391 @@ public void testCloseDoesNotFreePathScratchWhenWorkerStillAlive() throws Excepti }); } + /** + * Pins the claim-time registration gate at the top of + * {@code SegmentManager.serviceRing}: a snapshot entry whose ring was + * deregistered BEFORE the worker claims it must be skipped entirely — + * never claimed as {@code inService}, never handed to + * {@code serviceRing0}. This is the exact guarantee + * {@code CursorSendEngine.close()} relies on when it releases the ring, + * watermark and slot flock right after + * {@code awaitRingQuiescence(ring) == true}: the deregistering thread may + * already be freeing those resources, so a stale snapshot entry must not + * be touched at all (spare install, watermark write, drainTrimmable, or + * path building under the slot dir). + *

    + * Deterministic shape: three rings A, B, C registered before start, so + * the worker's first snapshot is exactly [A, B, C]. The worker parks in + * A's spare-install pass; while it is parked, B is deregistered (and a + * quiescence barrier for B must pass immediately — no pass for B is in + * flight). After release the worker walks the rest of its stale + * snapshot: it must skip B and service C. Every serviced ring is + * recorded from inside the worker's own pass (via the trim-sync hook + * reading {@code inService}), so the assertion is exact — no timing + * grace, no sleeps. + */ + @Test(timeout = 15_000L) + public void testStaleSnapshotEntrySkippedAfterDeregisterBeforeServiceClaim() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + 32); + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + SegmentRing[] rings = new SegmentRing[3]; + String[] slots = new String[3]; + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + // Rings the worker actually claimed and serviced, recorded from + // the worker thread itself at the trim-sync point every service + // pass reaches (spare needed or not). + java.util.Set serviced = + java.util.Collections.synchronizedSet( + java.util.Collections.newSetFromMap(new java.util.IdentityHashMap<>())); + boolean managerClosed = false; + try { + for (int i = 0; i < 3; i++) { + slots[i] = tmpDir + "/claim-skip-slot-" + i; + Assert.assertEquals(0, Files.mkdir(slots[i], Files.DIR_MODE_DEFAULT)); + MmapSegment initial = MmapSegment.create( + slots[i] + "/sf-initial.sfa", 0L, segSize); + rings[i] = new SegmentRing(initial, segSize); + } + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(10, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.setBeforeTrimSyncHook(() -> { + try { + Object ring = readInServiceRing(manager); + if (ring != null) { + serviced.add(ring); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + // Register all three BEFORE start: the worker's first snapshot + // is [A, B, C] and every fresh ring wants a hot spare, so the + // install hook parks the worker inside A's pass. + manager.register(rings[0], slots[0]); + manager.register(rings[1], slots[1]); + manager.register(rings[2], slots[2]); + manager.start(); + Assert.assertTrue("worker did not reach A's install pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // B is deregistered while its snapshot entry is still ahead of + // the worker's cursor. The quiescence barrier must pass at + // once: the in-flight pass is A's, not B's — this is the state + // in which an engine owner frees B's resources. + manager.deregister(rings[1]); + Assert.assertTrue("no pass for B is in flight — the barrier must pass immediately", + manager.awaitRingQuiescence(rings[1])); + + releaseWorker.countDown(); + + // Positive marker that the worker walked PAST B's snapshot + // slot: C sits after B in the same snapshot, so once C has + // been serviced, B's claim-or-skip decision has been made. + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!serviced.contains(rings[2])) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("worker never serviced ring C after release"); + } + Thread.sleep(1); + } + + Assert.assertTrue("ring A must have been serviced", serviced.contains(rings[0])); + Assert.assertFalse( + "worker claimed and serviced a snapshot entry that was deregistered " + + "before its pass started — the deregistering thread may already " + + "be releasing the ring/watermark/slot lock, so a stale snapshot " + + "entry must be skipped at claim time", + serviced.contains(rings[1])); + // Defense in depth: nothing may have been installed into the + // deregistered ring either. + Assert.assertNull("no hot spare may be installed into a deregistered ring", + readHotSpare(rings[1])); + + manager.close(); + managerClosed = true; + if (hookErr.get() != null) { + throw new AssertionError("worker-side hook failed", hookErr.get()); + } + } finally { + manager.setBeforeInstallSyncHook(null); + manager.setBeforeTrimSyncHook(null); + releaseWorker.countDown(); + if (!managerClosed) { + manager.close(); + } + for (SegmentRing ring : rings) { + if (ring != null) { + ring.close(); + } + } + } + }); + } + + /** + * Pins the scratch-handoff half of the timed-out-close contract in + * isolation: after {@code close()} gives up on the bounded join and hands + * {@code pathScratch} ownership to the worker, the WORKER's exit block + * alone must free the native buffer — with no retried {@code close()} + * ever running. The sibling test + * ({@link #testCloseDoesNotFreePathScratchWhenWorkerStillAlive}) retries + * {@code close()} before asserting the free, so a regression that dropped + * the worker-side free (leaving reclaim to a retry nobody is required to + * make) would stay green there: production owners do NOT retry — a + * timed-out close returns to the pool and the worker is the only thread + * left that can reclaim the allocation. + */ + @Test(timeout = 15_000L) + public void testWorkerAloneFreesPathScratchAfterTimedOutClose() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + 32); + String slot = tmpDir + "/worker-frees-scratch-slot"; + Assert.assertEquals(0, Files.mkdir(slot, Files.DIR_MODE_DEFAULT)); + MmapSegment initial = MmapSegment.create(slot + "/sf-initial.sfa", 0L, segSize); + SegmentRing ring = new SegmentRing(initial, segSize); + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + try { + manager.register(ring, slot); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(10, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + Assert.assertTrue("worker did not reach install hook", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Timed-out close: hands scratch ownership to the worker and + // returns. This is the last close() call this test makes. + manager.setWorkerJoinTimeoutMillis(50L); + manager.close(); + Thread worker = readWorkerThread(manager); + Assert.assertTrue("worker must still be live after the timed-out close", + worker != null && worker.isAlive()); + Assert.assertTrue("scratch must still be allocated while the worker may use it", + readPathScratchImpl(manager) != 0L); + + // Release the pass. running=false already, so the worker + // finishes the pass and exits — its exit block must free the + // handed-over scratch buffer without ANY further caller action. + releaseWorker.countDown(); + worker.join(TimeUnit.SECONDS.toMillis(10)); + Assert.assertFalse("worker never exited after release", worker.isAlive()); + Assert.assertEquals( + "worker exit block must free the handed-over path scratch — no retried " + + "close() runs in production after a timed-out close, so leaving " + + "the free to a retry leaks the native buffer for the process " + + "lifetime", + 0L, readPathScratchImpl(manager)); + + // Reap the dead thread for tidiness; must not double-free. + manager.close(); + Assert.assertNull("retried close must reap the exited worker", + readWorkerThread(manager)); + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + manager.close(); + ring.close(); + } + }); + } + + /** + * Pins the {@link SegmentManager#deferUntilWorkerExit} handoff contract + * that {@code CursorSendEngine.close()}'s slot-ownership transfer depends + * on: + *
      + *
    • rejects the handoff ({@code false}) when no worker ever started — + * the caller must clean up inline;
      • + *
    • accepts it ({@code true}) while the worker is live-but-slow + * mid service pass after a timed-out close(), and runs the cleanup + * exactly when the worker exits — never while the pass is still in + * flight;
      • + *
    • rejects it again once the worker loop has exited/been reaped.
      • + *
    + */ + @Test(timeout = 15_000L) + public void testDeferUntilWorkerExitRunsCleanupAfterFinalPass() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + 32); + String slot = tmpDir + "/defer-slot"; + Assert.assertEquals(0, Files.mkdir(slot, Files.DIR_MODE_DEFAULT)); + MmapSegment initial = MmapSegment.create(slot + "/sf-initial.sfa", 0L, segSize); + SegmentRing ring = new SegmentRing(initial, segSize); + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + CountDownLatch cleanupRan = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + boolean managerClosed = false; + try { + // Never-started manager: no worker will ever run the cleanup, + // and none can touch a slot — the caller must do it inline. + Assert.assertFalse("never-started manager must reject the handoff", + manager.deferUntilWorkerExit(cleanupRan::countDown)); + + manager.register(ring, slot); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(10, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + Assert.assertTrue("worker did not reach install hook", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Timed-out close: running=false, worker parked mid-pass — the + // exact state CursorSendEngine.close() hands ownership over in. + manager.setWorkerJoinTimeoutMillis(50L); + manager.close(); + Assert.assertFalse("worker must not be reaped while parked mid-pass", + manager.isWorkerReaped()); + + Assert.assertTrue("live-but-slow worker must accept the handoff", + manager.deferUntilWorkerExit(cleanupRan::countDown)); + Assert.assertEquals("cleanup must not run while the pass is still in flight", + 1, cleanupRan.getCount()); + + // Release the pass; the worker loop observes running=false, + // exits, and must run the deferred cleanup on its way out. + releaseWorker.countDown(); + Assert.assertTrue("cleanup never ran on worker exit", + cleanupRan.await(10, TimeUnit.SECONDS)); + + // Reap the exited worker, then: no live worker, no handoff. + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + manager.close(); + managerClosed = true; + Assert.assertFalse("reaped manager must reject the handoff", + manager.deferUntilWorkerExit(() -> { + })); + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + if (!managerClosed) { + Thread.interrupted(); + manager.close(); + } + ring.close(); + } + }); + } + + /** + * Pins the {@link SegmentManager#awaitRingQuiescence} contract that + * {@code CursorSendEngine.close()} depends on: + *
      + *
    • returns {@code false} (never {@code true}) while a service pass + * for the ring is provably in flight and the timeout elapses;
      • + *
    • preserves a pending caller interrupt without aborting;
      • + *
    • returns {@code true} once the in-flight pass has finished.
      • + *
    + */ + @Test(timeout = 15_000L) + public void testAwaitRingQuiescenceBlocksWhileServicePassInFlight() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segSize = MmapSegment.HEADER_SIZE + (MmapSegment.FRAME_HEADER_SIZE + 32); + String slot = tmpDir + "/quiesce-slot"; + Assert.assertEquals(0, Files.mkdir(slot, Files.DIR_MODE_DEFAULT)); + MmapSegment initial = MmapSegment.create(slot + "/sf-initial.sfa", 0L, segSize); + SegmentRing ring = new SegmentRing(initial, segSize); + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + boolean managerClosed = false; + try { + manager.register(ring, slot); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(10, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + Assert.assertTrue("worker did not reach install hook", + workerBlocked.await(5, TimeUnit.SECONDS)); + + manager.deregister(ring); + manager.setWorkerJoinTimeoutMillis(50L); + Thread.currentThread().interrupt(); + Assert.assertFalse( + "awaitRingQuiescence returned true while the worker was parked " + + "inside the service pass for this ring", + manager.awaitRingQuiescence(ring)); + Assert.assertTrue("awaitRingQuiescence must preserve the caller's interrupt", + Thread.interrupted()); + + releaseWorker.countDown(); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + Assert.assertTrue( + "awaitRingQuiescence must return true once the in-flight pass finished", + manager.awaitRingQuiescence(ring)); + + manager.close(); + managerClosed = true; + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + } finally { + manager.setBeforeInstallSyncHook(null); + releaseWorker.countDown(); + if (!managerClosed) { + Thread.interrupted(); + manager.close(); + } + ring.close(); + } + }); + } + @Test(timeout = 15_000L) public void testInterruptedCallerDoesNotAbandonReapableWorker() throws Exception { TestUtils.assertMemoryLeak(() -> { @@ -306,6 +691,24 @@ private static void cleanupRecursively(String dir) { } } + private static Object readHotSpare(SegmentRing ring) throws Exception { + Field f = SegmentRing.class.getDeclaredField("hotSpare"); + f.setAccessible(true); + return f.get(ring); + } + + private static Object readInServiceRing(SegmentManager manager) throws Exception { + Field inServiceF = SegmentManager.class.getDeclaredField("inService"); + inServiceF.setAccessible(true); + Object entry = inServiceF.get(manager); + if (entry == null) { + return null; + } + Field ringF = entry.getClass().getDeclaredField("ring"); + ringF.setAccessible(true); + return ringF.get(entry); + } + private static long readPathScratchImpl(SegmentManager manager) throws Exception { Field pathScratchF = SegmentManager.class.getDeclaredField("pathScratch"); pathScratchF.setAccessible(true); diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerPassBarrierBenchmark.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerPassBarrierBenchmark.java new file mode 100644 index 00000000..1bb6b899 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerPassBarrierBenchmark.java @@ -0,0 +1,131 @@ +/******************************************************************************* + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; + +/** + * Standalone comparison of the service-pass ownership barriers used by + * SegmentManager before and after its per-entry state change. This deliberately + * has no pass/fail threshold: it reports target-JVM costs while the production + * state machine and lifecycle tests establish correctness. + * + * 
    + * mvn -pl core test-compile
+ * mvn -pl core exec:java -Dexec.classpathScope=test \
+ *   -Dexec.mainClass=io.questdb.client.test.cutlass.qwp.client.sf.cursor.SegmentManagerPassBarrierBenchmark \
+ *   -Dexec.args="--passes=10000000"
+ *
    + */ +public final class SegmentManagerPassBarrierBenchmark { + + private static final Object MONITOR = new Object(); + private static final AtomicIntegerFieldUpdater STATE_UPDATER = + AtomicIntegerFieldUpdater.newUpdater(Entry.class, "state"); + private static volatile long checksum; + private static volatile int serviceProbe; + + public static void main(String[] args) { + int passes = 10_000_000; + for (String arg : args) { + if (arg.startsWith("--passes=")) { + passes = Integer.parseInt(arg.substring("--passes=".length())); + } else { + throw new IllegalArgumentException("unknown argument: " + arg); + } + } + if (passes < 1) { + throw new IllegalArgumentException("passes must be positive"); + } + + int warmup = Math.max(100_000, passes / 10); + for (int rings : new int[]{1, 32, 256}) { + measureMonitor(rings, warmup); + measureAtomic(rings, warmup); + long monitorNanos = measureMonitor(rings, passes); + long atomicNanos = measureAtomic(rings, passes); + System.out.printf( + "rings=%d passes=%d monitor(two enters)=%.2f ns/pass atomic(two CAS)=%.2f ns/pass ratio=%.2f%n", + rings, + passes, + (double) monitorNanos / passes, + (double) atomicNanos / passes, + (double) monitorNanos / atomicNanos); + } + System.out.println("checksum=" + checksum); + } + + private static Entry[] entries(int count) { + Entry[] entries = new Entry[count]; + for (int i = 0; i < count; i++) { + entries[i] = new Entry(); + } + return entries; + } + + private static long measureAtomic(int rings, int passes) { + Entry[] entries = entries(rings); + long start = System.nanoTime(); + for (int i = 0; i < passes; i++) { + Entry entry = entries[i % rings]; + if (!STATE_UPDATER.compareAndSet(entry, 0, 1)) { + throw new AssertionError("claim failed"); + } + service(entry); + if (!STATE_UPDATER.compareAndSet(entry, 1, 0)) { + throw new AssertionError("completion failed"); + } + } + checksum = checksum * 31 + entries[passes % rings].state; + return System.nanoTime() - start; + } + + private static long measureMonitor(int rings, int passes) { + Entry[] entries = entries(rings); + long start = System.nanoTime(); + for (int i = 0; i < passes; i++) { + Entry entry = entries[i % rings]; + synchronized (MONITOR) { + entry.state = 1; + } + service(entry); + synchronized (MONITOR) { + entry.state = 0; + } + } + checksum = checksum * 31 + entries[passes % rings].state; + return System.nanoTime() - start; + } + + private static void service(Entry entry) { + // Models the non-trivial serviceRing0 call between claim and complete + // and prevents the JVM from coarsening both monitor regions into one. + serviceProbe = entry.state; + } + + private static final class Entry { + volatile int state; + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java new file mode 100644 index 00000000..0feffe8a --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SegmentManagerUnlinkFailureTest.java @@ -0,0 +1,350 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.cutlass.qwp.client.sf.cursor; + +import io.questdb.client.cutlass.qwp.client.sf.cursor.AckWatermark; +import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentRing; +import io.questdb.client.std.Files; +import io.questdb.client.std.FilesFacade; +import io.questdb.client.std.MemoryTag; +import io.questdb.client.std.Unsafe; +import io.questdb.client.test.tools.TestUtils; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class SegmentManagerUnlinkFailureTest { + + private String tmpDir; + + @Before + public void setUp() { + tmpDir = Paths.get(System.getProperty("java.io.tmpdir"), + "qdb-segmgr-unlink-fault-" + System.nanoTime()).toString(); + Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT)); + } + + @After + public void tearDown() { + if (tmpDir != null) { + removeRecursive(tmpDir); + } + } + + @Test + public void testEnumerationFindNextFailureRefusesGenerationAllocation() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segmentSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L; + String dir = tmpDir + "/enumeration"; + Assert.assertEquals(0, Files.mkdir(dir, Files.DIR_MODE_DEFAULT)); + String lowerName = "sf-0000000000000000.sfa"; + String lowerPath = dir + "/" + lowerName; + String higherPath = dir + "/sf-0000000000000007.sfa"; + MmapSegment lower = MmapSegment.create(lowerPath, 0L, segmentSize); + lower.close(); + MmapSegment initial = MmapSegment.create(higherPath, 0L, segmentSize); + SegmentRing ring = new SegmentRing(initial, segmentSize); + byte[] originalHigher = java.nio.file.Files.readAllBytes(Paths.get(higherPath)); + FailingFilesFacade facade = new FailingFilesFacade(null, dir, lowerName); + try (SegmentManager manager = new SegmentManager( + segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 4, facade)) { + try { + manager.register(ring, dir); + Assert.fail("register accepted a partially enumerated SF directory"); + } catch (IllegalStateException expected) { + Assert.assertTrue(expected.getMessage().contains("could not fully enumerate")); + } + Assert.assertTrue("fault did not occur after the lower generation was observed", + facade.partialLowerObserved); + Assert.assertTrue("failed enumeration cursor was not closed", facade.partialFindClosed); + Assert.assertEquals("enumeration failure must not allocate or truncate a path", + 0, facade.openCleanCalls); + Assert.assertTrue("higher generation disappeared", Files.exists(higherPath)); + Assert.assertArrayEquals("partial enumeration changed the unseen higher generation", + originalHigher, java.nio.file.Files.readAllBytes(Paths.get(higherPath))); + } finally { + ring.close(); + } + }); + } + + @Test(timeout = 15_000L) + public void testFailedUnlinkRetainsBookkeepingAndUsesSuccessorPath() throws Exception { + TestUtils.assertMemoryLeak(() -> { + long segmentSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L; + String dir = tmpDir + "/unlink"; + Assert.assertEquals(0, Files.mkdir(dir, Files.DIR_MODE_DEFAULT)); + String failedPath = dir + "/sf-0000000000000000.sfa"; + String activePath = dir + "/sf-0000000000000001.sfa"; + MmapSegment initial = MmapSegment.create(failedPath, 0L, segmentSize); + SegmentRing ring = new SegmentRing(initial, segmentSize); + AckWatermark watermark = AckWatermark.open(dir); + Assert.assertNotNull(watermark); + watermark.write(-1L); + long payload = Unsafe.malloc(32, MemoryTag.NATIVE_DEFAULT); + try { + fill(payload, 32, (byte) 0x11); + Assert.assertEquals(0L, ring.appendOrFsn(payload, 32)); + ring.installHotSpare(MmapSegment.create(activePath, 1L, segmentSize)); + Assert.assertEquals(1L, ring.appendOrFsn(payload, 32)); + ring.acknowledge(0L); + byte[] original = java.nio.file.Files.readAllBytes(Paths.get(failedPath)); + + FailingFilesFacade facade = new FailingFilesFacade(failedPath, null, null); + try (SegmentManager manager = new SegmentManager( + segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 8, facade)) { + manager.register(ring, dir, watermark); + manager.start(); + Assert.assertTrue("manager never attempted the injected unlink", + facade.removeAttempted.await(5, TimeUnit.SECONDS)); + Assert.assertEquals("failed unlink must retain conservative registered bytes", + ring.totalSegmentBytes(), readTotalBytes(manager)); + } + + Assert.assertTrue("failed unlink path must remain observable", Files.exists(failedPath)); + Assert.assertTrue("failed unlink changed the acknowledged segment bytes", + Arrays.equals(original, java.nio.file.Files.readAllBytes(Paths.get(failedPath)))); + Assert.assertNotNull("failed unlink removed the segment from ring bookkeeping", + ring.firstSealed()); + Assert.assertEquals(failedPath, ring.firstSealed().path()); + Assert.assertEquals("watermark advanced although unlink did not commit", + -1L, watermark.read()); + + fill(payload, 32, (byte) 0x5A); + Assert.assertEquals("non-DEDUP successor must continue at the next FSN", + 2L, ring.appendOrFsn(payload, 32)); + String successorPath = ring.getActive().path(); + Assert.assertNotEquals("successor reused the acknowledged path", + failedPath, successorPath); + Assert.assertEquals(dir + "/sf-0000000000000002.sfa", successorPath); + Assert.assertTrue("successor segment was not created", Files.exists(successorPath)); + Assert.assertTrue("distinct non-DEDUP payload was not written to the successor", + containsRun(java.nio.file.Files.readAllBytes(Paths.get(successorPath)), + (byte) 0x5A, 32)); + Assert.assertTrue("successor write overwrote the failed-unlink segment", + Arrays.equals(original, java.nio.file.Files.readAllBytes(Paths.get(failedPath)))); + + try (SegmentManager retryManager = new SegmentManager( + segmentSize, TimeUnit.SECONDS.toNanos(60), segmentSize * 8, facade)) { + retryManager.register(ring, dir, watermark); + retryManager.start(); + retryManager.wakeWorker(); + awaitRetryCommit(failedPath, watermark); + } + MmapSegment firstSealed = ring.firstSealed(); + Assert.assertTrue("successful retry retained the acknowledged segment", + firstSealed == null || !failedPath.equals(firstSealed.path())); + } finally { + Unsafe.free(payload, 32, MemoryTag.NATIVE_DEFAULT); + ring.close(); + watermark.close(); + } + }); + } + + private static void awaitRetryCommit(String failedPath, AckWatermark watermark) { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (Files.exists(failedPath) || watermark.read() != 0L) { + if (System.nanoTime() > deadline) { + throw new AssertionError("unlink retry did not remove the file and advance the watermark"); + } + io.questdb.client.std.Compat.onSpinWait(); + } + } + + private static boolean containsRun(byte[] bytes, byte value, int length) { + int run = 0; + for (byte b : bytes) { + run = b == value ? run + 1 : 0; + if (run == length) { + return true; + } + } + return false; + } + + private static void fill(long address, int len, byte value) { + for (int i = 0; i < len; i++) { + Unsafe.getUnsafe().putByte(address + i, value); + } + } + + private static long readTotalBytes(SegmentManager manager) throws Exception { + Field field = SegmentManager.class.getDeclaredField("totalBytes"); + field.setAccessible(true); + Field lockField = SegmentManager.class.getDeclaredField("lock"); + lockField.setAccessible(true); + Object lock = lockField.get(manager); + synchronized (lock) { + return field.getLong(manager); + } + } + + private static void removeRecursive(String dir) { + long find = Files.findFirst(dir); + if (find > 0) { + try { + int rc = 1; + while (rc > 0) { + String name = Files.utf8ToString(Files.findName(find)); + if (name != null && !".".equals(name) && !"..".equals(name)) { + String child = dir + "/" + name; + if (!Files.remove(child)) { + removeRecursive(child); + } + } + rc = Files.findNext(find); + } + } finally { + Files.findClose(find); + } + } + Files.remove(dir); + } + + private static final class FailingFilesFacade implements FilesFacade { + private static final long PARTIAL_FIND_PTR = Long.MAX_VALUE; + private final String enumerationFailureDir; + private final String partialLowerName; + private final String unlinkFailurePath; + private final CountDownLatch removeAttempted = new CountDownLatch(1); + private int openCleanCalls; + private boolean partialFindClosed; + private long partialFindNamePtr; + private boolean partialLowerObserved; + private int unlinkFailuresRemaining = 1; + + private FailingFilesFacade( + String unlinkFailurePath, + String enumerationFailureDir, + String partialLowerName + ) { + this.unlinkFailurePath = unlinkFailurePath; + this.enumerationFailureDir = enumerationFailureDir; + this.partialLowerName = partialLowerName; + } + + @Override + public boolean allocate(int fd, long size) { return INSTANCE.allocate(fd, size); } + @Override + public long allocNativePath(String path) { return INSTANCE.allocNativePath(path); } + @Override + public int close(int fd) { return INSTANCE.close(fd); } + @Override + public boolean exists(String path) { return INSTANCE.exists(path); } + @Override + public void findClose(long findPtr) { + if (findPtr == PARTIAL_FIND_PTR) { + INSTANCE.freeNativePath(partialFindNamePtr); + partialFindNamePtr = 0L; + partialFindClosed = true; + } else { + INSTANCE.findClose(findPtr); + } + } + @Override + public long findFirst(String dir) { + if (dir.equals(enumerationFailureDir)) { + partialFindNamePtr = INSTANCE.allocNativePath(partialLowerName); + return PARTIAL_FIND_PTR; + } + return INSTANCE.findFirst(dir); + } + @Override + public long findName(long findPtr) { + return findPtr == PARTIAL_FIND_PTR ? partialFindNamePtr : INSTANCE.findName(findPtr); + } + @Override + public int findNext(long findPtr) { + if (findPtr == PARTIAL_FIND_PTR) { + partialLowerObserved = true; + return -1; + } + return INSTANCE.findNext(findPtr); + } + @Override + public int findType(long findPtr) { return INSTANCE.findType(findPtr); } + @Override + public void freeNativePath(long pathPtr) { INSTANCE.freeNativePath(pathPtr); } + @Override + public int fsync(int fd) { return INSTANCE.fsync(fd); } + @Override + public long length(int fd) { return INSTANCE.length(fd); } + @Override + public long length(String path) { return INSTANCE.length(path); } + @Override + public long length(long pathPtr) { return INSTANCE.length(pathPtr); } + @Override + public int lock(int fd) { return INSTANCE.lock(fd); } + @Override + public int mkdir(String path, int mode) { return INSTANCE.mkdir(path, mode); } + @Override + public int openCleanRW(String path) { + openCleanCalls++; + return INSTANCE.openCleanRW(path); + } + @Override + public int openCleanRW(long pathPtr) { + openCleanCalls++; + return INSTANCE.openCleanRW(pathPtr); + } + @Override + public int openRW(String path) { return INSTANCE.openRW(path); } + @Override + public int openRW(long pathPtr) { return INSTANCE.openRW(pathPtr); } + @Override + public long read(int fd, long addr, long len, long offset) { + return INSTANCE.read(fd, addr, len, offset); + } + @Override + public boolean remove(String path) { + if (path.equals(unlinkFailurePath) && unlinkFailuresRemaining > 0) { + unlinkFailuresRemaining--; + removeAttempted.countDown(); + return false; + } + return INSTANCE.remove(path); + } + @Override + public boolean remove(long pathPtr) { return INSTANCE.remove(pathPtr); } + @Override + public int rename(String oldPath, String newPath) { return INSTANCE.rename(oldPath, newPath); } + @Override + public boolean truncate(int fd, long size) { return INSTANCE.truncate(fd, size); } + @Override + public long write(int fd, long addr, long len, long offset) { + return INSTANCE.write(fd, addr, len, offset); + } + } +} diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SlotLockTest.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SlotLockTest.java index 644a7463..20930f50 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SlotLockTest.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/client/sf/cursor/SlotLockTest.java @@ -34,6 +34,7 @@ import java.nio.file.Paths; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -103,6 +104,71 @@ public void testCloseReleasesLock() throws Exception { }); } + @Test + public void testReleaseConfirmsAndIsIdempotent() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String slot = parentDir + "/verified-release"; + SlotLock lock = SlotLock.acquire(slot); + assertTrue("first release must confirm success", lock.release()); + // Idempotent: an already-released lock keeps reporting true — + // callers gating a "slot reusable" signal on it must never see + // a spurious false after a confirmed release. + assertTrue("repeat release must stay true", lock.release()); + // close() after release() is a safe no-op (QuietCloseable path). + lock.close(); + // Confirmed release means the slot is genuinely acquirable. + try (SlotLock again = SlotLock.acquire(slot)) { + assertEquals(slot, again.slotDir()); + } + }); + } + + /** + * The {@code release() == false} branch: when the OS reports an explicit + * unlock failure, release must (a) return {@code false} so owners gating a + * "slot reusable" signal never see a lie, (b) retain the fd because the + * non-consuming unlock can safely be retried, and (c) keep returning + * {@code false} while the failure persists. Once unlock succeeds, release + * confirms and stays confirmed. Swapping in a known-bad descriptor gives + * the slot-specific native primitive a deterministic unlock failure. + */ + @Test + public void testFailedUnlockRetainsFdAndReportsFalse() throws Exception { + TestUtils.assertMemoryLeak(() -> { + String slot = parentDir + "/failed-release"; + SlotLock lock = SlotLock.acquire(slot); + java.lang.reflect.Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int realFd = fdField.getInt(lock); + assertTrue("precondition: acquire must hold a live fd", realFd >= 0); + try { + // A non-negative descriptor no process has open makes the + // explicit flock/UnlockFileEx operation fail without consuming + // the real descriptor that continues to hold the slot lock. + fdField.setInt(lock, 1_000_000_000); + assertFalse("release must report false when explicit unlock fails", + lock.release()); + assertEquals("failed unlock must retain the fd for a safe retry", + 1_000_000_000, fdField.getInt(lock)); + assertFalse("repeat release must stay false while unlock keeps failing", + lock.release()); + // While the release is unconfirmed the real flock remains held. + try (SlotLock ignored = SlotLock.acquire(slot)) { + fail("slot must not be acquirable while the original flock fd is still open"); + } catch (IllegalStateException expected) { + // good - unconfirmed release really means "still locked". + } + } finally { + fdField.setInt(lock, realFd); + } + assertTrue("release must confirm once explicit unlock succeeds", lock.release()); + assertTrue("confirmed release must stay confirmed", lock.release()); + try (SlotLock again = SlotLock.acquire(slot)) { + assertEquals(slot, again.slotDir()); + } + }); + } + @Test public void testTwoDifferentSlotsCoexist() throws Exception { TestUtils.assertMemoryLeak(() -> { diff --git a/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java b/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java index 6d4c5ef0..139a445f 100644 --- a/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java +++ b/core/src/test/java/io/questdb/client/test/cutlass/qwp/websocket/TestWebSocketServer.java @@ -64,6 +64,8 @@ public class TestWebSocketServer implements Closeable { // client-side pool actually closed the connections it opened. private final AtomicInteger liveConnections = new AtomicInteger(); private final int port; + private final AtomicInteger roleRejectCount = new AtomicInteger(); + private final CountDownLatch roleRejectLatch = new CountDownLatch(1); private final AtomicBoolean running = new AtomicBoolean(false); private final ServerSocket serverSocket; private final CountDownLatch startLatch = new CountDownLatch(1); @@ -165,6 +167,10 @@ public TestWebSocketServer(WebSocketServerHandler handler, this.port = serverSocket.getLocalPort(); } + public boolean awaitRoleReject(long timeout, TimeUnit unit) throws InterruptedException { + return roleRejectLatch.await(timeout, unit); + } + public boolean awaitStart(long timeout, TimeUnit unit) throws InterruptedException { return startLatch.await(timeout, unit); } @@ -221,6 +227,13 @@ public int liveConnectionCount() { return liveConnections.get(); } + /** + * Number of HTTP 421 role-reject responses sent over the server's lifetime. + */ + public int roleRejectCount() { + return roleRejectCount.get(); + } + /** * Replaces the advertised role for subsequent handshakes (live update). */ @@ -586,6 +599,8 @@ private boolean performHandshake() throws IOException { "\r\n"; out.write(sb.getBytes(StandardCharsets.US_ASCII)); out.flush(); + roleRejectCount.incrementAndGet(); + roleRejectLatch.countDown(); return false; } diff --git a/core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java b/core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java index cfef2feb..896dc69a 100644 --- a/core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java +++ b/core/src/test/java/io/questdb/client/test/impl/SenderPoolSfTest.java @@ -30,7 +30,11 @@ import ch.qos.logback.core.read.ListAppender; import io.questdb.client.Sender; import io.questdb.client.cutlass.line.LineSenderException; +import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender; +import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine; import io.questdb.client.cutlass.qwp.client.sf.cursor.OrphanScanner; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager; +import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock; import io.questdb.client.impl.PooledSender; import io.questdb.client.impl.SenderPool; import io.questdb.client.std.Files; @@ -48,11 +52,13 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.file.Paths; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -658,6 +664,617 @@ public void testSlotLeakedWhenDelegateCloseDoesNotReleaseFlockDuringReap() throw }); } + @Test + public void testRetiredSlotRecoveredByHousekeeperAfterLateFlockRelease() throws Exception { + // Recovery twin of testSlotLeakedWhenDelegateCloseDoesNotReleaseFlock: + // a slot retired because close() returned with the flock still held is + // NOT lost until process exit. Engine cleanup may complete later on a + // worker/I/O-thread exit path (isSlotLockReleased() re-probes the + // retained engine), and the housekeeper's reapIdle() tick must then + // return the index to the free set: leakedSlots back down, slotInUse + // cleared, full capacity restored. + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool(config, 1, 2, 500, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender a = pool.borrow(); + Assert.assertTrue(Files.exists(slot("default-0"))); + + // Forge the retire: real teardown first (no native leaks), + // then clear slotLockReleased so discardBroken retires the + // slot as leaked. + Sender delegate = getDelegate(a); + delegate.close(); + setBooleanField(delegate, "slotLockReleased", false); + invokeDiscardBroken(pool, a); + Assert.assertEquals("precondition: one slot must be retired", + 1, pool.leakedSlotCount()); + + // Forge the late release: the deferred cleanup finished and + // the delegate now reports the flock dropped (in production + // this flip comes from isSlotLockReleased() re-probing the + // retained engine after the manager worker exited). + setBooleanField(delegate, "slotLockReleased", true); + + // The housekeeper tick is the recovery driver. + pool.reapIdle(); + + Assert.assertEquals("recovered slot must leave the leaked count", + 0, pool.leakedSlotCount()); + boolean[] slotInUse = (boolean[]) getField(pool, "slotInUse"); + Assert.assertFalse("recovered slot index 0 must return to the free set", + slotInUse[0]); + + // Full capacity restored: with maxSize=2, two concurrent + // borrows must succeed again (index 0 is reusable — its + // flock is genuinely free). + PooledSender b = pool.borrow(); + PooledSender c = pool.borrow(); + try { + Assert.assertEquals(2, countSlotDirs()); + } finally { + c.close(); + b.close(); + } + } + } + }); + } + + @Test + public void testCapacityStarvedBorrowRecoversRetiredSlot() throws Exception { + // Borrow-path twin of the housekeeper recovery test: a borrow that + // would otherwise park on the cap check must re-probe retired slots + // before waiting, so a late flock release converts a guaranteed + // borrow timeout into an immediate creation on the recovered index — + // no housekeeper tick required. + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool(config, 1, 2, 500, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender a = pool.borrow(); + Assert.assertTrue(Files.exists(slot("default-0"))); + + Sender delegate = getDelegate(a); + delegate.close(); + setBooleanField(delegate, "slotLockReleased", false); + invokeDiscardBroken(pool, a); + Assert.assertEquals("precondition: one slot must be retired", + 1, pool.leakedSlotCount()); + + // One live borrow + one retired slot = cap reached. + PooledSender b = pool.borrow(); + Assert.assertTrue(Files.exists(slot("default-1"))); + try { + // Late release lands while the pool is capacity-starved. + setBooleanField(delegate, "slotLockReleased", true); + + // The next borrow hits the cap check, re-probes, frees + // index 0, and must create on it instead of timing out. + PooledSender c = pool.borrow(); + try { + Assert.assertEquals("borrow must recover the retired slot's capacity", + 0, pool.leakedSlotCount()); + Assert.assertEquals(2, countSlotDirs()); + } finally { + c.close(); + } + } finally { + b.close(); + } + } + } + }); + } + + @Test(timeout = 30_000) + public void testDeferredFlockReleaseWakesParkedLongTimeoutBorrower() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + server.getPort() + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool( + config, 1, 1, 60_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender lease = pool.borrow(); + Sender delegate = getDelegate(lease); + CursorSendEngine engine = (CursorSendEngine) getField(delegate, "cursorEngine"); + SlotLock slotLock = (SlotLock) getField(engine, "slotLock"); + Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int realFd = fdField.getInt(slotLock); + Assert.assertTrue("precondition: live flock fd", realFd >= 0); + + CountDownLatch borrowerAcquired = new CountDownLatch(1); + CountDownLatch borrowerParked = new CountDownLatch(1); + CountDownLatch releaseBorrower = new CountDownLatch(1); + AtomicReference borrowerFailure = new AtomicReference<>(); + AtomicReference recovered = new AtomicReference<>(); + Thread borrower = new Thread(() -> { + try { + recovered.set(pool.borrow()); + borrowerAcquired.countDown(); + if (!releaseBorrower.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("timed out waiting to release borrower"); + } + } catch (Throwable t) { + borrowerFailure.compareAndSet(null, t); + } finally { + PooledSender sender = recovered.get(); + if (sender != null) { + sender.close(); + } + } + }, "sender-pool-deferred-release-waiter"); + + try { + synchronized (slotLock) { + fdField.setInt(slotLock, 1_000_000_000); + } + invokeDiscardBroken(pool, lease); + Assert.assertEquals("failed release must retire the only slot", + 1, pool.leakedSlotCount()); + Assert.assertFalse(engine.isCloseCompleted()); + + pool.setBeforeBorrowWaitHook(borrowerParked::countDown); + borrower.start(); + Assert.assertTrue("borrower must reach the condition wait with a long timeout", + borrowerParked.await(5, TimeUnit.SECONDS)); + // The hook runs under the pool lock immediately before awaitNanos. + // Acquiring that lock here proves awaitNanos atomically enqueued the + // borrower and released the lock before restoration can start. This + // read-only operation neither changes pool state nor signals a waiter. + pool.availableSize(); + + // This restored fd is the only source of progress: the retry driver + // confirms the release. There is no housekeeper or pool mutation. + synchronized (slotLock) { + fdField.setInt(slotLock, realFd); + } + Assert.assertTrue("deferred flock release must wake the parked borrower", + borrowerAcquired.await(5, TimeUnit.SECONDS)); + Assert.assertNull("borrower must not fail", borrowerFailure.get()); + Assert.assertEquals("release wakeup must recover retired capacity", + 0, pool.leakedSlotCount()); + } finally { + pool.setBeforeBorrowWaitHook(null); + releaseBorrower.countDown(); + if (!engine.isCloseCompleted()) { + synchronized (slotLock) { + fdField.setInt(slotLock, realFd); + } + } + borrower.join(TimeUnit.SECONDS.toMillis(1)); + if (borrower.isAlive()) { + borrower.interrupt(); + borrower.join(TimeUnit.SECONDS.toMillis(5)); + } + Assert.assertFalse("borrower thread must finish", borrower.isAlive()); + } + if (borrowerFailure.get() != null) { + throw new AssertionError("borrower failed", borrowerFailure.get()); + } + } + } + }); + } + + @Test + public void testMixedRetiredSlotsRecoverWithoutLosingAccounting() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool( + config, 0, 8, 500, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender[] leases = new PooledSender[8]; + Sender[] delegates = new Sender[8]; + for (int i = 0; i < leases.length; i++) { + leases[i] = pool.borrow(); + delegates[i] = getDelegate(leases[i]); + } + for (int i = 0; i < leases.length; i++) { + // Release the real native resources, then forge the + // delayed publication which makes the pool retire the + // slot. The idempotent second close in discardBroken() + // leaves the forged state unchanged. + delegates[i].close(); + setBooleanField(delegates[i], "slotLockReleased", false); + invokeDiscardBroken(pool, leases[i]); + } + Assert.assertEquals(8, pool.leakedSlotCount()); + + // Mix completed and incomplete entries throughout the + // retired list. A recovery pass must remove exactly these + // four without skipping a swapped entry or corrupting the + // bitmap/count relationship. + int[] released = {0, 2, 5, 7}; + for (int i = 0; i < released.length; i++) { + setBooleanField(delegates[released[i]], "slotLockReleased", true); + } + pool.reapIdle(); + + Assert.assertEquals(4, pool.leakedSlotCount()); + Assert.assertEquals(4, ((List) getField(pool, "retiredSlots")).size()); + boolean[] slotInUse = (boolean[]) getField(pool, "slotInUse"); + for (int i = 0; i < slotInUse.length; i++) { + boolean mustRemainRetired = i == 1 || i == 3 || i == 4 || i == 6; + Assert.assertEquals("slot reservation mismatch at index " + i, + mustRemainRetired, slotInUse[i]); + } + + // Exactly the four restored reservations are reusable. + PooledSender[] recovered = new PooledSender[4]; + try { + for (int i = 0; i < recovered.length; i++) { + recovered[i] = pool.borrow(); + } + Assert.assertEquals(4, pool.totalSize()); + Assert.assertEquals(4, pool.leakedSlotCount()); + } finally { + for (int i = 0; i < recovered.length; i++) { + if (recovered[i] != null) { + recovered[i].close(); + } + } + } + } + } + }); + } + + @Test(timeout = 30_000) + public void testMultipleWaitingBorrowersWakeForStaggeredRetiredSlotRecovery() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool( + config, 0, 6, 10_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender[] leases = new PooledSender[6]; + Sender[] delegates = new Sender[6]; + for (int i = 0; i < leases.length; i++) { + leases[i] = pool.borrow(); + delegates[i] = getDelegate(leases[i]); + } + for (int i = 0; i < leases.length; i++) { + delegates[i].close(); + setBooleanField(delegates[i], "slotLockReleased", false); + invokeDiscardBroken(pool, leases[i]); + } + Assert.assertEquals("all capacity must start retired", + 6, pool.leakedSlotCount()); + + final int borrowerCount = 3; + CountDownLatch allAcquired = new CountDownLatch(borrowerCount); + CountDownLatch allDone = new CountDownLatch(borrowerCount); + CountDownLatch firstAcquired = new CountDownLatch(1); + CountDownLatch initialWaiters = new CountDownLatch(borrowerCount); + CountDownLatch releaseBorrowers = new CountDownLatch(1); + CountDownLatch reparkedWaiters = new CountDownLatch(borrowerCount - 1); + AtomicReference failure = new AtomicReference<>(); + ConcurrentHashMap initialWaiterThreads = new ConcurrentHashMap<>(); + ConcurrentHashMap reparkedWaiterThreads = new ConcurrentHashMap<>(); + PooledSender[] recovered = new PooledSender[borrowerCount]; + pool.setBeforeBorrowWaitHook(() -> { + if (initialWaiterThreads.putIfAbsent(Thread.currentThread(), Boolean.TRUE) == null) { + initialWaiters.countDown(); + } + }); + for (int i = 0; i < borrowerCount; i++) { + final int borrower = i; + Thread thread = new Thread(() -> { + try { + recovered[borrower] = pool.borrow(); + allAcquired.countDown(); + firstAcquired.countDown(); + if (!releaseBorrowers.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("timed out waiting to release recovered borrower"); + } + } catch (Throwable e) { + failure.compareAndSet(null, e); + } finally { + try { + if (recovered[borrower] != null) { + recovered[borrower].close(); + } + } catch (Throwable e) { + failure.compareAndSet(null, e); + } finally { + allDone.countDown(); + } + } + }, "sender-pool-retired-waiter-" + i); + thread.start(); + } + + try { + Assert.assertTrue("all borrowers must reach the condition wait", + initialWaiters.await(5, TimeUnit.SECONDS)); + Assert.assertEquals("three distinct borrowers must reach the wait path", + borrowerCount, initialWaiterThreads.size()); + + // The hook runs while each borrower holds the pool lock, + // immediately before awaitNanos atomically releases that + // lock and enqueues it. Once the last latch count lands, + // the first reapIdle() cannot acquire the lock until all + // three borrowers are definitely condition waiters. + pool.setBeforeBorrowWaitHook(() -> { + if (reparkedWaiterThreads.putIfAbsent(Thread.currentThread(), Boolean.TRUE) == null) { + reparkedWaiters.countDown(); + } + }); + setBooleanField(delegates[2], "slotLockReleased", true); + pool.reapIdle(); + + // One restored index admits exactly one borrower. The + // other two must consume signalAll(), lose the capacity + // race, and deterministically re-enter the wait path. + Assert.assertTrue("one borrower must take the first restored index", + firstAcquired.await(5, TimeUnit.SECONDS)); + Assert.assertTrue("two distinct borrowers must re-park after the first recovery", + reparkedWaiters.await(5, TimeUnit.SECONDS)); + Assert.assertEquals("exactly two distinct borrowers must re-enter the wait path", + borrowerCount - 1, reparkedWaiterThreads.size()); + Assert.assertEquals("exactly one borrower should hold restored capacity", + borrowerCount - 1, allAcquired.getCount()); + + // Recover two non-contiguous entries in one reverse / + // swap-remove pass. A single signal() here would wake + // only one of the two proven waiters; signalAll() must + // wake both and let them claim the two restored indices. + pool.setBeforeBorrowWaitHook(null); + setBooleanField(delegates[0], "slotLockReleased", true); + setBooleanField(delegates[5], "slotLockReleased", true); + pool.reapIdle(); + Assert.assertTrue("all waiting borrowers must receive restored capacity", + allAcquired.await(5, TimeUnit.SECONDS)); + Assert.assertEquals(3, pool.totalSize()); + Assert.assertEquals(3, pool.leakedSlotCount()); + + boolean[] seen = new boolean[6]; + for (int i = 0; i < recovered.length; i++) { + int slotIndex = getIntField(slotOf(recovered[i]), "slotIndex"); + Assert.assertFalse("borrowers must receive distinct restored indices", + seen[slotIndex]); + seen[slotIndex] = true; + } + Assert.assertTrue("slot 0 must be restored", seen[0]); + Assert.assertTrue("slot 2 must be restored", seen[2]); + Assert.assertTrue("slot 5 must be restored", seen[5]); + if (failure.get() != null) { + throw new AssertionError("waiting borrower failed", failure.get()); + } + } finally { + pool.setBeforeBorrowWaitHook(null); + releaseBorrowers.countDown(); + Assert.assertTrue("borrower threads must finish", + allDone.await(10, TimeUnit.SECONDS)); + } + if (failure.get() != null) { + throw new AssertionError("waiting borrower failed", failure.get()); + } + } + } + }); + } + + @Test + public void testPoolRetiresAndRecoversSlotThroughFailedFlockReleaseRetry() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool(config, 1, 1, 2_000, + Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender a = pool.borrow(); + Sender delegate = getDelegate(a); + CursorSendEngine engine = (CursorSendEngine) getField(delegate, "cursorEngine"); + SlotLock slotLock = (SlotLock) getField(engine, "slotLock"); + Field fdField = SlotLock.class.getDeclaredField("fd"); + fdField.setAccessible(true); + int realFd = fdField.getInt(slotLock); + Assert.assertTrue("precondition: live flock fd", realFd >= 0); + try { + // Inject one persistent explicit-unlock failure. + // Delegate close must retire the only pool slot rather + // than publish a release while the real flock remains held. + synchronized (slotLock) { + fdField.setInt(slotLock, 1_000_000_000); + } + invokeDiscardBroken(pool, a); + Assert.assertEquals("failed release must retire pool capacity", + 1, pool.leakedSlotCount()); + Assert.assertFalse(engine.isCloseCompleted()); + + // Remove the fault without calling close again: the + // engine's error-path retry driver runs outside the + // pool lock and must eventually publish completion. + synchronized (slotLock) { + fdField.setInt(slotLock, realFd); + } + long deadlineNs = System.nanoTime() + TimeUnit.SECONDS.toNanos(10); + while (!engine.isCloseCompleted()) { + if (System.nanoTime() > deadlineNs) { + throw new AssertionError("flock-release retry never completed"); + } + Thread.sleep(1L); + } + + // A capacity-starved public borrow re-probes the + // retained delegate, frees index 0, and proves the + // recovered flock is genuinely acquirable. + PooledSender recovered = pool.borrow(); + try { + Assert.assertEquals("retry must restore pool capacity", + 0, pool.leakedSlotCount()); + Assert.assertEquals(1, countSlotDirs()); + } finally { + recovered.close(); + } + } finally { + if (!engine.isCloseCompleted()) { + synchronized (slotLock) { + fdField.setInt(slotLock, realFd); + Assert.assertTrue("restored fd must release cleanly", + slotLock.release()); + } + } + } + } + } + }); + } + + @Test + public void testPoolRetiresAndRecoversSlotThroughRealManagerWorkerWedge() throws Exception { + // Full-stack twin of the two forged-flag recovery tests above: no + // reflection-forged slotLockReleased anywhere. The REAL mechanism is + // driven end to end — the delegate's owned SegmentManager worker is + // wedged mid service pass (test hook), the delegate's close() takes + // the real timed-out-join → worker-exit handoff path, the pool + // retires the slot off the delegate's genuine isSlotLockReleased() + // report, the worker's deferred cleanup releases the real flock, and + // the housekeeper re-probe restores capacity — proving the recovered + // index is genuinely reusable by borrowing on it again (a forged flag + // would pass the accounting asserts but collide on the flock here). + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + try (SenderPool pool = new SenderPool(config, 1, 2, 500, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender a = pool.borrow(); + Assert.assertTrue(Files.exists(slot("default-0"))); + + Sender delegate = getDelegate(a); + CursorSendEngine engine = (CursorSendEngine) getField(delegate, "cursorEngine"); + Assert.assertNotNull("SF delegate must own a cursor engine", engine); + SegmentManager manager = (SegmentManager) getField(engine, "manager"); + + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + try { + // Park the manager worker inside a service pass for + // the delegate's ring. The trim-sync point is reached + // on every ~1ms tick, so this wedges deterministically. + manager.setBeforeTrimSyncHook(() -> { + if (!fired.compareAndSet(false, true)) return; + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, new AssertionError( + "timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + Assert.assertTrue("manager worker never entered a service pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + + // Real teardown against the wedged worker: the + // delegate's engine close times out its bounded join, + // hands cleanup to the worker's exit path, and reports + // the retained flock; the pool must retire the slot. + // Fault the old callback-allocation/registration path. + // C3 makes the owned-engine handoff preallocated, so + // this hook must no longer be reached by production + // teardown. + manager.setBeforeExitCleanupRegistrationHook(() -> { + throw new OutOfMemoryError("simulated callback allocation failure"); + }); + manager.setWorkerJoinTimeoutMillis(50L); + invokeDiscardBroken(pool, a); + Assert.assertEquals( + "pool must retire the slot while the delegate's manager " + + "worker holds the deferred cleanup", + 1, pool.leakedSlotCount()); + Assert.assertFalse("engine cleanup must still be pending", + engine.isCloseCompleted()); + + // Un-wedge and deterministically reap the manager. + // No sender/engine close retry is allowed: worker exit + // itself must own and finish the preallocated handoff. + releaseWorker.countDown(); + manager.close(); + Assert.assertTrue("manager worker must be reaped", manager.isWorkerReaped()); + Assert.assertTrue("worker exit must run deferred cleanup despite the " + + "old allocation fault injection", + engine.isCloseCompleted()); + + // The housekeeper tick is the recovery driver. + pool.reapIdle(); + Assert.assertEquals("recovered slot must leave the leaked count", + 0, pool.leakedSlotCount()); + boolean[] slotInUse = (boolean[]) getField(pool, "slotInUse"); + Assert.assertFalse("recovered slot index 0 must return to the free set", + slotInUse[0]); + + // The proof a forged flag cannot fake: both indices — + // including the recovered one, whose flock was really + // dropped by the worker-exit cleanup — admit live + // senders again. + PooledSender b = pool.borrow(); + PooledSender c = pool.borrow(); + try { + Assert.assertEquals(2, countSlotDirs()); + } finally { + c.close(); + b.close(); + } + if (hookErr.get() != null) { + throw new AssertionError("trim hook failed", hookErr.get()); + } + } finally { + manager.setBeforeExitCleanupRegistrationHook(null); + manager.setBeforeTrimSyncHook(null); + releaseWorker.countDown(); + } + } + } + }); + } + + @Test + public void testPreallocatedExitHandoffCleansInRangeStartupRecoverer() throws Exception { + assertPreallocatedExitHandoffCleansStartupRecoverer(0, 1); + } + + @Test + public void testPreallocatedExitHandoffCleansOutOfRangeStartupRecoverer() throws Exception { + assertPreallocatedExitHandoffCleansStartupRecoverer(1, 1); + } + // ---------------------------------------------------------------------- // Recovery: stable slot ids let a re-created pool re-adopt unacked data. // ---------------------------------------------------------------------- @@ -757,12 +1374,98 @@ public void testRecoveryDelegateForcesOffInitialConnectMode() throws Exception { }); } + @Test + public void testSharedManagerPassCompletionRecoversRetiredPoolSlot() throws Exception { + TestUtils.assertMemoryLeak(() -> { + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer server = new TestWebSocketServer(handler)) { + int port = server.getPort(); + server.start(); + Assert.assertTrue(server.awaitStart(5, TimeUnit.SECONDS)); + + long segSize = io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment.HEADER_SIZE + + io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment.FRAME_HEADER_SIZE + 32L; + SegmentManager manager = new SegmentManager(segSize, TimeUnit.SECONDS.toNanos(60)); + CountDownLatch cleanupFinished = new CountDownLatch(1); + CountDownLatch workerBlocked = new CountDownLatch(1); + CountDownLatch releaseWorker = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + AtomicReference hookErr = new AtomicReference<>(); + AtomicReference engineRef = new AtomicReference<>(); + manager.setAfterRingCleanupHook(cleanupFinished::countDown); + manager.setBeforeInstallSyncHook(() -> { + if (!fired.compareAndSet(false, true)) { + return; + } + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting for test to release worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + manager.start(); + Assert.assertEquals(0, Files.mkdir(sfDir, Files.DIR_MODE_DEFAULT)); + String config = "ws::addr=localhost:" + port + ";sf_dir=" + sfDir + ";"; + IntFunction factory = slotIndex -> { + CursorSendEngine engine = new CursorSendEngine( + slot("default-" + slotIndex), segSize, manager); + engineRef.set(engine); + QwpWebSocketSender sender = QwpWebSocketSender.createForTesting("localhost", port); + sender.setCursorEngine(engine, true); + return sender; + }; + SenderPool pool = new SenderPool( + config, 0, 1, 500, 0, Long.MAX_VALUE, factory); + try { + PooledSender lease = pool.borrow(); + Assert.assertTrue("shared manager never entered the sender ring's service pass", + workerBlocked.await(5, TimeUnit.SECONDS)); + lease.close(); + + manager.setWorkerJoinTimeoutMillis(50L); + pool.reapIdle(); + Assert.assertEquals("pool must retire the slot while its shared-manager pass is live", + 1, pool.leakedSlotCount()); + CursorSendEngine engine = engineRef.get(); + Assert.assertFalse("engine cleanup must remain pending during the live pass", + engine.isCloseCompleted()); + + releaseWorker.countDown(); + Assert.assertTrue("deferred cleanup did not finish with the ring pass", + cleanupFinished.await(5, TimeUnit.SECONDS)); + if (hookErr.get() != null) { + throw new AssertionError("install hook failed", hookErr.get()); + } + + pool.reapIdle(); + Assert.assertEquals("pass completion must drive deferred engine cleanup and restore capacity", + 0, pool.leakedSlotCount()); + Assert.assertTrue("deferred cleanup must publish the released flock", + engine.isCloseCompleted()); + try (PooledSender recovered = pool.borrow()) { + Assert.assertTrue("recovered slot must be reusable", Files.exists(slot("default-0"))); + } + } finally { + releaseWorker.countDown(); + manager.setAfterRingCleanupHook(null); + manager.setBeforeInstallSyncHook(null); + pool.close(); + manager.close(); + } + } + }); + } + @Test public void testStartupRecoveryRetiresSlotWhenRecovererCloseLeavesFlockHeld() throws Exception { // C1 regression: the startup recovery loop MUST mirror discardBroken / // reapIdle. When a recoverer's delegate close() returns with the SF // flock still held (the I/O thread refused to stop), the recovered slot - // index must be retired permanently (leakedSlots++, slotInUse stays + // index must be retired (leakedSlots++, slotInUse stays // set) -- NOT freed. Freeing it would let a later borrow re-pick the // still-locked dir and resurrect "sf slot already in use", the exact // failure class this PR exists to kill. Pre-fix the recovery finally @@ -838,7 +1541,7 @@ public void testStartupRecoveryRetiresSlotWhenRecovererCloseLeavesFlockHeld() th try { Assert.assertTrue("borrow must use a fresh slot dir", Files.exists(slot("default-1"))); - // Capacity is permanently reduced by the leaked slot: + // Capacity stays reduced while the flock is held: // max=2, one leaked + one live => the next borrow times // out rather than colliding on the locked dir. try { @@ -866,6 +1569,292 @@ public void testStartupRecoveryRetiresSlotWhenRecovererCloseLeavesFlockHeld() th }); } + @Test + public void testStartupRetiredSlotRecoveredAfterLateFlockRelease() throws Exception { + // Recovery twin of + // testStartupRecoveryRetiresSlotWhenRecovererCloseLeavesFlockHeld: a + // slot retired by STARTUP recovery (recoverer close() returned with + // the flock still held) must not stay lost until process exit. + // isSlotLockReleased() is no longer a one-shot snapshot -- deferred + // engine cleanup on a worker exit path can release the flock later -- + // so the pool must keep the recoverer in retiredSlots and re-probe it. + // Pre-fix, startup recovery only ticked leakedSlots and dropped the + // recoverer: at maxSize=1 every later borrow timed out forever even + // after the flock dropped. This test is RED until the recoverer is + // retained. + TestUtils.assertMemoryLeak(() -> { + // Phase 1: strand unacked data under default-0 so startup recovery + // treats it as a candidate orphan and builds a recoverer. + try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) { + int silentPort = silent.getPort(); + silent.start(); + Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS)); + String cfg1 = "ws::addr=localhost:" + silentPort + ";sf_dir=" + sfDir + + ";close_flush_timeout_millis=500;"; + try (SenderPool pool = new SenderPool(cfg1, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender s = pool.borrow(); + for (int i = 0; i < 3; i++) { + s.table("recover").longColumn("v", i).atNow(); + s.flush(); + } + s.close(); + } + } + Assert.assertTrue("unacked data must persist under default-0", + hasSegmentFile(slot("default-0"))); + + // Phase 2: maxSize=1 -- the worst case, where the single slot's + // retirement starves the whole pool. Forge the retention exactly + // like the retire test (closed=true makes the recovery drain and + // close a no-op, so the real flock stays held and slotLockReleased + // stays false), but only for the FIRST build of slot 0: the + // post-recovery borrow below must get a working delegate on the + // recovered index. + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer ack = new TestWebSocketServer(handler)) { + int ackPort = ack.getPort(); + ack.start(); + Assert.assertTrue(ack.awaitStart(5, TimeUnit.SECONDS)); + String cfg2 = "ws::addr=localhost:" + ackPort + ";sf_dir=" + sfDir + ";"; + + AtomicReference forged = new AtomicReference<>(); + IntFunction factory = idx -> { + Sender real = Sender.builder(cfg2).senderId("default-" + idx).build(); + if (idx == 0 && forged.compareAndSet(null, real)) { + try { + setBooleanField(real, "closed", true); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + return real; + }; + + try (SenderPool pool = newPoolWithFactory(cfg2, 0, 1, 500, factory)) { + Assert.assertNotNull("recovery must have built slot 0", forged.get()); + Assert.assertEquals("precondition: startup recovery must retire the slot", + 1, pool.leakedSlotCount()); + + // While the flock is genuinely held, borrows time out: the + // cap-check re-probe finds the flock still reported held. + try { + pool.borrow(); + Assert.fail("borrow must time out while the slot is retired"); + } catch (LineSenderException e) { + Assert.assertTrue(e.getMessage(), e.getMessage().contains("timed out")); + } + + // The late release: the "wedged worker" finishes and the + // flock genuinely drops (un-forge and close the recoverer + // for real; in production this flip comes from + // isSlotLockReleased() re-probing the retained engine after + // the worker exited). + Sender recoverer = forged.get(); + setBooleanField(recoverer, "closed", false); + recoverer.close(); + + // The capacity-starved borrow must re-probe the startup- + // retired slot, recover its capacity, and create on the + // freed index -- no housekeeper tick required. + PooledSender b = pool.borrow(); + try { + Assert.assertEquals("borrow must recover the startup-retired slot's capacity", + 0, pool.leakedSlotCount()); + boolean[] slotInUse = (boolean[]) getField(pool, "slotInUse"); + Assert.assertTrue("recovered index 0 must carry the new borrow", slotInUse[0]); + Assert.assertEquals(1, countSlotDirs()); + } finally { + b.close(); + } + } + } + }); + } + + @Test + public void testZeroTimeoutBorrowProbesRetiredSlotBeforeThrowing() throws Exception { + // Boundary twin of testStartupRetiredSlotRecoveredAfterLateFlockRelease: + // acquireTimeoutMillis=0 is a valid try-once borrow (builder rejects only + // < 0). Pre-fix, borrow() ran the terminal timeout check BEFORE + // reprobeRetiredSlots(), so a zero-budget borrow threw "timed out" + // without its one probe -- even when the retired slot's flock had + // already dropped and a probe would have restored capacity and admitted + // a creation. Deterministic: no housekeeper runs in this test, so + // borrow() is the only reprobe driver. Recovery is driven manually via + // the deferred-pool step helper because the inline path reuses + // acquireTimeoutMillis as its recovery budget -- 0 would skip recovery + // outright. This test is RED until the probe is hoisted above the + // timeout check. + TestUtils.assertMemoryLeak(() -> { + // Phase 1: strand unacked data under default-0 so startup recovery + // treats it as a candidate orphan and builds a recoverer. + try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) { + int silentPort = silent.getPort(); + silent.start(); + Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS)); + String cfg1 = "ws::addr=localhost:" + silentPort + ";sf_dir=" + sfDir + + ";close_flush_timeout_millis=500;"; + try (SenderPool pool = new SenderPool(cfg1, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender s = pool.borrow(); + for (int i = 0; i < 3; i++) { + s.table("recover").longColumn("v", i).atNow(); + s.flush(); + } + s.close(); + } + } + Assert.assertTrue("unacked data must persist under default-0", + hasSegmentFile(slot("default-0"))); + + // Phase 2: maxSize=1, zero acquire budget, deferred recovery driven + // step-by-step (the housekeeper's per-tick unit, budgeted by + // RECOVERY_DRAIN_BUDGET_MILLIS, not the zero acquire budget). Forge + // the retirement (closed=true makes the recovery drain and close a + // no-op, so the real flock stays held), then release the flock for + // real BEFORE borrowing: the recovery is discoverable, but only via + // a probe. + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer ack = new TestWebSocketServer(handler)) { + int ackPort = ack.getPort(); + ack.start(); + Assert.assertTrue(ack.awaitStart(5, TimeUnit.SECONDS)); + String cfg2 = "ws::addr=localhost:" + ackPort + ";sf_dir=" + sfDir + ";"; + + AtomicReference forged = new AtomicReference<>(); + IntFunction factory = idx -> { + Sender real = Sender.builder(cfg2).senderId("default-" + idx).build(); + if (idx == 0 && forged.compareAndSet(null, real)) { + try { + setBooleanField(real, "closed", true); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + return real; + }; + + try (SenderPool pool = newDeferredPoolWithFactory(cfg2, 0, 1, 0, factory)) { + //noinspection StatementWithEmptyBody + while (invokeRunStartupRecoveryStep(pool)) { + // drive the whole backlog, one housekeeper-tick unit at a time + } + Assert.assertNotNull("recovery must have built slot 0", forged.get()); + Assert.assertEquals("precondition: startup recovery must retire the slot", + 1, pool.leakedSlotCount()); + + // The late release: un-forge and close the recoverer for + // real. The flock genuinely drops, but nothing signals the + // pool -- the release happens in the delegate, volatile + // writes only. + Sender recoverer = forged.get(); + setBooleanField(recoverer, "closed", false); + recoverer.close(); + + // Try-once borrow: its single pass must probe, recover the + // capacity, and create on the freed index. Pre-fix this + // threw "timed out" without ever probing. + PooledSender b = pool.borrow(); + try { + Assert.assertEquals("zero-timeout borrow must recover the retired slot's capacity", + 0, pool.leakedSlotCount()); + } finally { + b.close(); + } + } + } + }); + } + + @Test + public void testParkedBorrowerGetsFinalProbeAfterBudgetExpiry() throws Exception { + // Positive-timeout twin of the zero-timeout test. A borrower parks in + // awaitNanos while the retired slot's flock is genuinely held and + // sleeps out its full budget. A test hook releases the flock after the + // wait reports expiry but before the terminal loop pass. Pre-fix that + // pass hit the timeout check before reprobeRetiredSlots() and threw -- + // missing capacity that had already come back. Post-fix the terminal + // pass probes first, recovers the index, and admits the creation. + TestUtils.assertMemoryLeak(() -> { + // Phase 1: strand unacked data under default-0. + try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) { + int silentPort = silent.getPort(); + silent.start(); + Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS)); + String cfg1 = "ws::addr=localhost:" + silentPort + ";sf_dir=" + sfDir + + ";close_flush_timeout_millis=500;"; + try (SenderPool pool = new SenderPool(cfg1, 1, 1, 1_000, Long.MAX_VALUE, Long.MAX_VALUE)) { + PooledSender s = pool.borrow(); + for (int i = 0; i < 3; i++) { + s.table("recover").longColumn("v", i).atNow(); + s.flush(); + } + s.close(); + } + } + Assert.assertTrue("unacked data must persist under default-0", + hasSegmentFile(slot("default-0"))); + + // Phase 2: maxSize=1 and a positive acquire budget. A test hook + // releases the flock only after awaitNanos() has returned with that + // budget exhausted, so the terminal wake-up pass is deterministic. + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer ack = new TestWebSocketServer(handler)) { + int ackPort = ack.getPort(); + ack.start(); + Assert.assertTrue(ack.awaitStart(5, TimeUnit.SECONDS)); + String cfg2 = "ws::addr=localhost:" + ackPort + ";sf_dir=" + sfDir + ";"; + + AtomicReference forged = new AtomicReference<>(); + IntFunction factory = idx -> { + Sender real = Sender.builder(cfg2).senderId("default-" + idx).build(); + if (idx == 0 && forged.compareAndSet(null, real)) { + try { + setBooleanField(real, "closed", true); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + return real; + }; + + try (SenderPool pool = newPoolWithFactory(cfg2, 0, 1, 100, factory)) { + Assert.assertNotNull("recovery must have built slot 0", forged.get()); + Assert.assertEquals("precondition: startup recovery must retire the slot", + 1, pool.leakedSlotCount()); + + AtomicBoolean waitExpired = new AtomicBoolean(); + pool.setBorrowWaitExpiredHook(() -> { + Assert.assertTrue("expired-wait hook must run exactly once", + waitExpired.compareAndSet(false, true)); + Sender recoverer = forged.get(); + try { + setBooleanField(recoverer, "closed", false); + } catch (Exception e) { + throw new RuntimeException(e); + } + // The flock drops only after the positive awaitNanos() + // budget is exhausted. This delegate-side release does + // not signal slotReleased. + recoverer.close(); + }); + try { + PooledSender b = pool.borrow(); + try { + Assert.assertTrue("borrow must exhaust its positive wait budget", + waitExpired.get()); + Assert.assertEquals("wake-up probe must recover the retired slot's capacity", + 0, pool.leakedSlotCount()); + } finally { + b.close(); + } + } finally { + pool.setBorrowWaitExpiredHook(null); + } + } + } + }); + } + // ---------------------------------------------------------------------- // Concurrency stress: borrow/return churn must never collide on a slot. // ---------------------------------------------------------------------- @@ -1306,6 +2295,84 @@ public void testDefaultConfigRecoversOutOfRangeSlotsAfterShrink() throws Excepti }); } + @Test + public void testFailedOutOfRangeRecoveryRetriesAfterPrimaryReturns() throws Exception { + // A deferred pool can already have its sole in-range sender borrowed when + // startup recovery reaches an out-of-range slot left by a larger pool. + // A transient recoverer build failure must leave that candidate pending: + // after the primary lease returns, the SAME pool must retry and drain it. + TestUtils.assertMemoryLeak(() -> { + try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) { + silent.start(); + Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS)); + String seedConfig = "ws::addr=localhost:" + silent.getPort() + ";sf_dir=" + sfDir + + ";sender_id=default-1;close_flush_timeout_millis=0;"; + try (Sender seed = Sender.fromConfig(seedConfig)) { + seed.table("recover").longColumn("v", 1L).atNow(); + seed.flush(); + } + } + Assert.assertTrue("out-of-range fixture must contain unacked data", + hasSegmentFile(slot("default-1"))); + + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer ack = new TestWebSocketServer(handler)) { + ack.start(); + Assert.assertTrue(ack.awaitStart(5, TimeUnit.SECONDS)); + String config = "ws::addr=localhost:" + ack.getPort() + ";sf_dir=" + sfDir + ";"; + AtomicBoolean primaryReturned = new AtomicBoolean(); + AtomicInteger recoveryAttempts = new AtomicInteger(); + IntFunction factory = idx -> { + if (idx == 1) { + recoveryAttempts.incrementAndGet(); + if (!primaryReturned.get()) { + throw new LineSenderException("transient out-of-range recovery failure"); + } + } + return Sender.builder(config).senderId("default-" + idx).build(); + }; + + try (SenderPool pool = newDeferredPoolWithFactory(config, 0, 1, 5_000, factory)) { + PooledSender primary = pool.borrow(); + Object primarySlot = slotOf(primary); + + Assert.assertFalse("first recovery attempt must stop at the transient failure", + invokeRunStartupRecoveryStep(pool)); + Assert.assertEquals("exactly one out-of-range recovery attempt", 1, + recoveryAttempts.get()); + Assert.assertTrue("failed recovery must preserve the candidate", + hasSegmentFile(slot("default-1"))); + Assert.assertEquals("out-of-range failure must not consume in-range capacity", + 0, pool.leakedSlotCount()); + Assert.assertTrue("out-of-range recoverer must not enter retired-slot bookkeeping", + ((List) getField(pool, "retiredSlots")).isEmpty()); + + primary.close(); + primaryReturned.set(true); + invokeRunStartupRecoveryOnce(pool); + + Assert.assertEquals("same live pool must retry the out-of-range candidate", 2, + recoveryAttempts.get()); + Assert.assertFalse("retry must drain the preserved out-of-range data", + hasSegmentFile(slot("default-1"))); + Assert.assertTrue("retry must deliver the recovered frame", handler.frames.get() >= 1); + Assert.assertEquals("successful out-of-range retry must not consume capacity", + 0, pool.leakedSlotCount()); + Assert.assertTrue("out-of-range retry must leave retired slots untouched", + ((List) getField(pool, "retiredSlots")).isEmpty()); + + PooledSender next = pool.borrow(); + try { + Assert.assertSame("normal borrow must reuse the returned primary slot", + primarySlot, slotOf(next)); + } finally { + next.close(); + } + } + } + }); + } + @Test public void testInRangeIdleSlotIsRecoveredAtStartupUnderSteadyLowLoad() throws Exception { // The drain exclusion is bounded to [0, maxSize) so a sibling's drainer @@ -1806,6 +2873,134 @@ private String slot(String name) { return sfDir + "/" + name; } + private void assertPreallocatedExitHandoffCleansStartupRecoverer( + int strandedIndex, int maxSize) throws Exception { + TestUtils.assertMemoryLeak(() -> { + String strandedId = "default-" + strandedIndex; + try (TestWebSocketServer silent = new TestWebSocketServer(new SilentHandler())) { + silent.start(); + Assert.assertTrue(silent.awaitStart(5, TimeUnit.SECONDS)); + String config = "ws::addr=localhost:" + silent.getPort() + ";sf_dir=" + sfDir + + ";close_flush_timeout_millis=100;"; + Sender sender = Sender.builder(config).senderId(strandedId).build(); + sender.table("recover").longColumn("v", strandedIndex).atNow(); + sender.flush(); + try { + sender.close(); + } catch (LineSenderException expected) { + Assert.assertTrue(expected.getMessage(), + expected.getMessage().contains("drain timed out")); + } + } + Assert.assertTrue("startup-recovery fixture must contain an unacked segment", + hasSegmentFile(slot(strandedId))); + + CountingAckHandler handler = new CountingAckHandler(); + try (TestWebSocketServer ack = new TestWebSocketServer(handler)) { + ack.start(); + Assert.assertTrue(ack.awaitStart(5, TimeUnit.SECONDS)); + String config = "ws::addr=localhost:" + ack.getPort() + ";sf_dir=" + sfDir + ";"; + AtomicBoolean instrumented = new AtomicBoolean(); + AtomicReference engineRef = new AtomicReference<>(); + AtomicReference managerRef = new AtomicReference<>(); + AtomicReference hookErr = new AtomicReference<>(); + CountDownLatch releaseWorker = new CountDownLatch(1); + IntFunction factory = idx -> { + Sender sender = Sender.builder(config).senderId("default-" + idx).build(); + if (idx == strandedIndex && instrumented.compareAndSet(false, true)) { + try { + CursorSendEngine engine = (CursorSendEngine) getField(sender, "cursorEngine"); + SegmentManager manager = (SegmentManager) getField(engine, "manager"); + CountDownLatch workerBlocked = new CountDownLatch(1); + AtomicBoolean fired = new AtomicBoolean(); + manager.setBeforeTrimSyncHook(() -> { + if (!fired.compareAndSet(false, true)) { + return; + } + workerBlocked.countDown(); + try { + if (!releaseWorker.await(20, TimeUnit.SECONDS)) { + hookErr.compareAndSet(null, + new AssertionError("timed out waiting to release recovery worker")); + } + } catch (Throwable t) { + hookErr.compareAndSet(null, t); + } + }); + if (!workerBlocked.await(5, TimeUnit.SECONDS)) { + throw new AssertionError("recovery manager never entered a service pass"); + } + manager.setBeforeExitCleanupRegistrationHook(() -> { + throw new OutOfMemoryError("simulated callback allocation failure"); + }); + manager.setWorkerJoinTimeoutMillis(50L); + engineRef.set(engine); + managerRef.set(manager); + } catch (Throwable t) { + try { + sender.close(); + } catch (Throwable ignored) { + } + throw new RuntimeException(t); + } + } + return sender; + }; + + SenderPool pool = null; + try { + pool = newPoolWithFactory(config, 0, maxSize, 2_000, factory); + CursorSendEngine engine = engineRef.get(); + SegmentManager manager = managerRef.get(); + Assert.assertNotNull("startup recovery must build the stranded slot", engine); + if (strandedIndex < maxSize) { + Assert.assertEquals("in-range recoverer must remain retired while worker is live", + 1, pool.leakedSlotCount()); + } else { + Assert.assertEquals("out-of-range recovery must not consume pool capacity", + 0, pool.leakedSlotCount()); + } + Assert.assertFalse("cleanup must remain pending while the worker is live", + engine.isCloseCompleted()); + + releaseWorker.countDown(); + manager.close(); + Assert.assertTrue("recovery manager worker must be reaped", manager.isWorkerReaped()); + Assert.assertTrue("worker exit must complete startup-recoverer cleanup without " + + "a sender or engine close retry [index=" + strandedIndex + "]", + engine.isCloseCompleted()); + if (hookErr.get() != null) { + throw new AssertionError("recovery worker hook failed", hookErr.get()); + } + if (strandedIndex < maxSize) { + pool.reapIdle(); + Assert.assertEquals("late cleanup must restore in-range pool capacity", + 0, pool.leakedSlotCount()); + } + try (SlotLock ignored = SlotLock.acquire(slot(strandedId))) { + // Completion must mean the real slot flock is reusable. + } + } finally { + releaseWorker.countDown(); + SegmentManager manager = managerRef.get(); + if (manager != null) { + manager.setBeforeExitCleanupRegistrationHook(null); + manager.setBeforeTrimSyncHook(null); + manager.setWorkerJoinTimeoutMillis(TimeUnit.SECONDS.toMillis(60)); + manager.close(); + } + CursorSendEngine engine = engineRef.get(); + if (engine != null && !engine.isCloseCompleted()) { + engine.close(); + } + if (pool != null) { + pool.close(); + } + } + } + }); + } + private int countSlotDirs() { if (!Files.exists(sfDir)) { return 0; diff --git a/core/src/test/java/io/questdb/client/test/network/SocketTrafficShutdownTest.java b/core/src/test/java/io/questdb/client/test/network/SocketTrafficShutdownTest.java new file mode 100644 index 00000000..47d168f9 --- /dev/null +++ b/core/src/test/java/io/questdb/client/test/network/SocketTrafficShutdownTest.java @@ -0,0 +1,463 @@ +/*+***************************************************************************** + * ___ _ ____ ____ + * / _ \ _ _ ___ ___| |_| _ \| __ ) + * | | | | | | |/ _ \/ __| __| | | | _ \ + * | |_| | |_| | __/\__ \ |_| |_| | |_) | + * \__\_\\__,_|\___||___/\__|____/|____/ + * + * Copyright (c) 2014-2019 Appsicle + * Copyright (c) 2019-2026 QuestDB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ + +package io.questdb.client.test.network; + +import io.questdb.client.network.JavaTlsClientSocketFactory; +import io.questdb.client.network.Kqueue; +import io.questdb.client.network.KqueueFacade; +import io.questdb.client.network.KqueueFacadeImpl; +import io.questdb.client.network.NetworkFacade; +import io.questdb.client.network.NetworkFacadeImpl; +import io.questdb.client.network.PlainSocket; +import io.questdb.client.network.Socket; +import io.questdb.client.network.SocketReadinessWaiter; +import io.questdb.client.network.TlsSessionInitFailedException; +import io.questdb.client.std.Os; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Test; +import org.slf4j.LoggerFactory; + +import javax.net.ssl.SSLContext; +import java.lang.reflect.Field; +import java.lang.reflect.Proxy; +import java.net.InetSocketAddress; +import java.net.ServerSocket; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +public class SocketTrafficShutdownTest { + private static final NetworkFacade NF = NetworkFacadeImpl.INSTANCE; + + private static class CompatibilityNetworkFacade implements NetworkFacade { + private final AtomicInteger closeCount; + + private CompatibilityNetworkFacade(AtomicInteger closeCount) { + this.closeCount = closeCount; + } + + @Override + public int close(int fd) { + closeCount.incrementAndGet(); + return 0; + } + + @Override + public void close(int fd, org.slf4j.Logger logger) { + close(fd); + } + + @Override + public void configureKeepAlive(int fd) { + } + + @Override + public int configureNonBlocking(int fd) { + return 0; + } + + @Override + public int connect(int fd, long pSockaddr) { + return 0; + } + + @Override + public int connectAddrInfo(int fd, long pAddrInfo) { + return 0; + } + + @Override + public int connectAddrInfoTimeout(int fd, long pAddrInfo, int timeoutMillis) { + return 0; + } + + @Override + public int errno() { + return 0; + } + + @Override + public void freeAddrInfo(long pAddrInfo) { + } + + @Override + public void freeSockAddr(long pSockaddr) { + } + + @Override + public long getAddrInfo(CharSequence host, int port) { + return 0; + } + + @Override + public int getSndBuf(int fd) { + return 0; + } + + @Override + public int recvRaw(int fd, long buffer, int bufferLen) { + return 0; + } + + @Override + public int sendRaw(int fd, long buffer, int bufferLen) { + return 0; + } + + @Override + public int sendToRaw(int fd, long lo, int len, long socketAddress) { + return 0; + } + + @Override + public int sendToRawScatter(int fd, long segmentsPtr, int segmentCount, long socketAddress) { + return 0; + } + + @Override + public int setMulticastInterface(int fd, int ipv4Address) { + return 0; + } + + @Override + public int setMulticastTtl(int fd, int ttl) { + return 0; + } + + @Override + public boolean setSndBuf(int fd, int size) { + return false; + } + + @Override + public int setTcpNoDelay(int fd, boolean noDelay) { + return 0; + } + + @Override + public long sockaddr(int address, int port) { + return 0; + } + + @Override + public int socketTcp(boolean blocking) { + return 0; + } + + @Override + public int socketUdp() { + return 0; + } + + @Override + public boolean testConnection(int fd, long buffer, int bufferSize) { + return false; + } + } + + private static class CompatibilitySocket implements Socket { + private final AtomicInteger closeCount; + + private CompatibilitySocket(AtomicInteger closeCount) { + this.closeCount = closeCount; + } + + @Override + public void close() { + closeCount.incrementAndGet(); + } + + @Override + public int getFd() { + return -1; + } + + @Override + public boolean isClosed() { + return false; + } + + @Override + public void of(int fd) { + } + + @Override + public int recv(long bufferPtr, int bufferLen) { + return -1; + } + + @Override + public int send(long bufferPtr, int bufferLen) { + return -1; + } + + @Override + public void startTlsSession(CharSequence peerName, SocketReadinessWaiter waiter) throws TlsSessionInitFailedException { + } + + @Override + public boolean supportsTls() { + return false; + } + + @Override + public int tlsIO(int readinessFlags) { + return 0; + } + + @Override + public boolean wantsTlsWrite() { + return false; + } + } + + @Test + public void testCompatibilityDefaultsDoNotBypassCustomTransportOwnership() { + AtomicInteger facadeCloseCount = new AtomicInteger(); + NetworkFacade customFacade = new CompatibilityNetworkFacade(facadeCloseCount); + + PlainSocket plainSocket = new PlainSocket(customFacade, LoggerFactory.getLogger(SocketTrafficShutdownTest.class)); + plainSocket.of(42); + assertUnsupported(plainSocket::closeTraffic); + Assert.assertEquals("facade compatibility fallback must not release a synthetic descriptor", + 0, facadeCloseCount.get()); + Assert.assertEquals(42, plainSocket.getFd()); + plainSocket.close(); + Assert.assertEquals(1, facadeCloseCount.get()); + + AtomicInteger socketCloseCount = new AtomicInteger(); + Socket customSocket = new CompatibilitySocket(socketCloseCount); + assertUnsupported(customSocket::closeTraffic); + Assert.assertEquals("socket compatibility fallback must not run destructive close", + 0, socketCloseCount.get()); + } + + @Test(timeout = 30_000L) + public void testPlainSocketShutdownWakesMacOsKqueueAndRetainsFd() throws Exception { + assertShutdownWakesMacOsKqueue(new PlainSocket(NF, LoggerFactory.getLogger(SocketTrafficShutdownTest.class))); + } + + @Test + public void testTlsSocketTrafficGatePreservesTlsStateUntilFullClose() throws Exception { + AtomicInteger closeCount = new AtomicInteger(); + AtomicInteger shutdownCount = new AtomicInteger(); + NetworkFacade facade = (NetworkFacade) Proxy.newProxyInstance( + NetworkFacade.class.getClassLoader(), + new Class[]{NetworkFacade.class}, + (proxy, method, args) -> { + if ("close".equals(method.getName())) { + closeCount.incrementAndGet(); + return method.getReturnType() == int.class ? 0 : null; + } + if ("shutdown".equals(method.getName())) { + shutdownCount.incrementAndGet(); + return 0; + } + if (method.getReturnType() == boolean.class) { + return false; + } + if (method.getReturnType() == int.class) { + return 0; + } + if (method.getReturnType() == long.class) { + return 0L; + } + return null; + } + ); + Socket socket = JavaTlsClientSocketFactory.INSECURE_NO_VALIDATION.newInstance( + facade, + LoggerFactory.getLogger(SocketTrafficShutdownTest.class) + ); + socket.of(42); + Field sslEngineField = socket.getClass().getDeclaredField("sslEngine"); + Field stateField = socket.getClass().getDeclaredField("state"); + sslEngineField.setAccessible(true); + stateField.setAccessible(true); + sslEngineField.set(socket, SSLContext.getDefault().createSSLEngine()); + stateField.setInt(socket, 2); // JavaTlsClientSocket.STATE_TLS + Object[] tlsState = snapshotTlsState(socket); + + try { + socket.closeTraffic(); + + Object[] stateAfterTrafficClose = snapshotTlsState(socket); + for (int i = 0; i < tlsState.length; i++) { + if (i == 2) { + Assert.assertEquals("traffic cancellation must preserve TLS state", tlsState[i], stateAfterTrafficClose[i]); + } else { + Assert.assertSame("traffic cancellation must preserve SSLEngine/buffer references", + tlsState[i], stateAfterTrafficClose[i]); + } + } + Assert.assertEquals(1, shutdownCount.get()); + Assert.assertEquals("traffic cancellation must not release the delegate fd", 0, closeCount.get()); + Assert.assertEquals(42, socket.getFd()); + Assert.assertFalse(socket.isClosed()); + } finally { + // Restore a valid plaintext state so full close does not attempt a + // synthetic TLS close_notify with uninitialised session buffers. + sslEngineField.set(socket, null); + stateField.setInt(socket, 1); // JavaTlsClientSocket.STATE_PLAINTEXT + socket.close(); + } + Assert.assertEquals(1, closeCount.get()); + Assert.assertTrue(socket.isClosed()); + } + + @Test(timeout = 30_000L) + public void testTlsSocketTrafficGateUsesDelegateShutdownAndRetainsFd() throws Exception { + assertShutdownWakesMacOsKqueue(JavaTlsClientSocketFactory.INSECURE_NO_VALIDATION.newInstance( + NF, + LoggerFactory.getLogger(SocketTrafficShutdownTest.class) + )); + } + + private static void assertShutdownWakesMacOsKqueue(Socket socket) throws Exception { + Assume.assumeTrue("real kqueue cancellation coverage runs on macOS", Os.type == Os.DARWIN); + + AtomicBoolean pollDone = new AtomicBoolean(); + AtomicInteger pollResult = new AtomicInteger(Integer.MIN_VALUE); + AtomicReference pollFailure = new AtomicReference<>(); + CountDownLatch pollEntered = new CountDownLatch(1); + KqueueFacade facade = new KqueueFacade() { + private final KqueueFacade delegate = KqueueFacadeImpl.INSTANCE; + + @Override + public NetworkFacade getNetworkFacade() { + return delegate.getNetworkFacade(); + } + + @Override + public int kevent(int kq, long changeList, int nChanges, long eventList, int nEvents, int timeout) { + if (eventList != 0 && nEvents > 0) { + pollEntered.countDown(); + } + return delegate.kevent(kq, changeList, nChanges, eventList, nEvents, timeout); + } + + @Override + public int kqueue() { + return delegate.kqueue(); + } + }; + + int fd = -1; + Thread waiter = null; + try (ServerSocket listener = new ServerSocket()) { + listener.bind(new InetSocketAddress("127.0.0.1", 0)); + long addrInfo = NF.getAddrInfo("127.0.0.1", listener.getLocalPort()); + Assert.assertNotEquals(-1L, addrInfo); + try { + fd = NF.socketTcp(true); + Assert.assertTrue("could not allocate client socket", fd >= 0); + Assert.assertEquals(0, NF.connectAddrInfoTimeout(fd, addrInfo, 5_000)); + } finally { + NF.freeAddrInfo(addrInfo); + } + + try (java.net.Socket peer = listener.accept(); Kqueue kqueue = new Kqueue(facade, 1)) { + try { + Assert.assertEquals(0, NF.configureNonBlocking(fd)); + socket.of(fd); + int retainedFd = fd; + fd = -1; + + kqueue.setWriteOffset(0); + kqueue.readFD(retainedFd, 0); + Assert.assertEquals(0, kqueue.register(1)); + + waiter = new Thread(() -> { + try { + pollResult.set(kqueue.poll(10_000)); + } catch (Throwable t) { + pollFailure.set(t); + } finally { + pollDone.set(true); + } + }, "socket-traffic-kqueue-waiter"); + waiter.start(); + + Assert.assertTrue("waiter did not enter the native kqueue wait", + pollEntered.await(5, TimeUnit.SECONDS)); + Assert.assertFalse("peer unexpectedly made the read wait ready", pollDone.get()); + + socket.closeTraffic(); + + waiter.join(TimeUnit.SECONDS.toMillis(5)); + Assert.assertFalse("shutdown did not wake the native kqueue wait", waiter.isAlive()); + Assert.assertNull("native kqueue wait failed", pollFailure.get()); + Assert.assertTrue("shutdown must produce a readiness event", pollResult.get() > 0); + Assert.assertEquals("traffic cancellation must retain fd ownership", retainedFd, socket.getFd()); + Assert.assertFalse("traffic cancellation must not perform full close", socket.isClosed()); + + socket.close(); + Assert.assertTrue("full close must release the retained fd", socket.isClosed()); + } finally { + socket.closeTraffic(); + if (waiter != null) { + waiter.join(TimeUnit.SECONDS.toMillis(5)); + } + socket.close(); + } + } + } finally { + if (fd != -1) { + NF.close(fd); + } + } + } + + private static void assertUnsupported(Runnable operation) { + try { + operation.run(); + Assert.fail("expected unsupported traffic shutdown"); + } catch (UnsupportedOperationException expected) { + // Expected compatibility behavior: no native or destructive fallback. + } + } + + private static Object[] snapshotTlsState(Socket socket) throws Exception { + String[] fieldNames = { + "callerOutputBuffer", + "sslEngine", + "state", + "unwrapInputBuffer", + "unwrapOutputBuffer", + "wrapInputBuffer", + "wrapOutputBuffer" + }; + Object[] state = new Object[fieldNames.length]; + for (int i = 0; i < fieldNames.length; i++) { + Field field = socket.getClass().getDeclaredField(fieldNames[i]); + field.setAccessible(true); + state[i] = field.get(socket); + } + return state; + } +}