Skip to content

Commit 7df62d5

Browse files
committed
test(qwp): close the remaining engine/manager shutdown coverage gaps
Deterministic regression tests for the shutdown paths the coverage review flagged as untested: - SegmentManagerCloseRaceTest#testStaleSnapshotEntrySkippedAfterDeregisterBeforeServiceClaim: a snapshot entry deregistered before the worker claims it must be skipped at claim time (serviced rings recorded from the worker's own trim-sync point via inService, so the assertion is exact -- no sleeps). - SegmentManagerCloseRaceTest#testWorkerAloneFreesPathScratchAfterTimedOutClose: after a timed-out close hands pathScratch to the worker, the worker's exit block alone must free it -- no retried close() runs in production, so the sibling test's retry-then-assert shape masked a regression here. - CursorSendEngineSlotReacquisitionTest#testTerminalCleanupRunsExactlyOnceWhenRetriedCloseRacesWorkerHandoff: a retried close() racing the worker parked mid-finishClose must lose the terminalCleanupClaimed CAS -- no double cleanup, no premature completion, flock untouched until the worker's release. - CursorSendEngineSlotReacquisitionTest#testMemoryModeOwnedCloseHandsCleanupToWorkerExit: memory-mode (null sfDir/slotLock/watermark) timed-out close must take the same worker-exit handoff without NPE and free the ring's native memory. - EngineClosePublishAfterFlockReleaseTest#testUnconfirmedFlockReleaseKeepsCloseIncomplete: a failed flock release must never publish closeCompleted, and a retried close() must neither throw nor fabricate completion. - SlotLockTest#testFailedCloseRetainsFdAndReportsFalse: release()==false retains the fd for retry and keeps reporting false while the failure persists. - SlotLockReleasedContractTest#testDelegatedIoThreadEngineCloseFlipsSlotLockReleased: the delegated-I/O-close branch (delegateEngineClose()==true) must retain the engine so isSlotLockReleased() flips true once the I/O thread's exit path releases the flock; the existing forged I/O-refusal test throws from delegateEngineClose() before the retained-engine assignment and can never reach this branch. Mutation-verified: dropping the retainedEngine assignment fails the test with the pinned message.
1 parent d990470 commit 7df62d5

5 files changed

Lines changed: 782 additions & 0 deletions

File tree

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/SlotLockReleasedContractTest.java

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@
2424

2525
package io.questdb.client.test.cutlass.qwp.client;
2626

27+
import io.questdb.client.DefaultHttpClientConfiguration;
2728
import io.questdb.client.Sender;
29+
import io.questdb.client.cutlass.http.client.WebSocketClient;
2830
import io.questdb.client.cutlass.qwp.client.QwpWebSocketSender;
2931
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
3032
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop;
33+
import io.questdb.client.network.PlainSocketFactory;
3134
import io.questdb.client.cutlass.qwp.client.sf.cursor.MmapSegment;
3235
import io.questdb.client.cutlass.qwp.client.sf.cursor.SegmentManager;
3336
import io.questdb.client.cutlass.qwp.client.sf.cursor.SlotLock;
@@ -305,6 +308,162 @@ public void testSlotLockNotReleasedUntilManagerWorkerQuiesces() throws Exception
305308
});
306309
}
307310

311+
/**
312+
* Delegated-I/O-close recovery path: when {@code close()} bails early
313+
* because the I/O thread refuses to stop, the engine close is delegated
314+
* to that thread's exit path ({@code delegateEngineClose()} returned
315+
* true) and the sender must retain the engine for re-probing —
316+
* {@code isSlotLockReleased()} stays false while the thread lives, then
317+
* flips true the moment the thread's exit path completes the engine
318+
* close and releases the flock. Pre-fix, this branch discarded the
319+
* engine reference, so the late release was permanently invisible to a
320+
* pool that had retired the slot. The existing forged I/O-refusal test
321+
* can never reach this branch: its sabotaged loop throws from
322+
* {@code delegateEngineClose()} itself (nulled latch), before the
323+
* retained-engine assignment.
324+
* <p>
325+
* The wedge is the same deterministic one CursorWebSocketSendLoop's C5
326+
* test uses: the I/O thread sits in a blocking "connect" (a
327+
* ReconnectFactory immune to unpark, never interrupted by loop.close()),
328+
* and the closer thread's pre-set interrupt flag makes
329+
* {@code shutdownLatch.await()} throw immediately — the real production
330+
* path to {@code ioThreadStopped = false}.
331+
*/
332+
@Test(timeout = 30_000L)
333+
public void testDelegatedIoThreadEngineCloseFlipsSlotLockReleased() throws Exception {
334+
TestUtils.assertMemoryLeak(() -> {
335+
String tmpDir = Paths.get(System.getProperty("java.io.tmpdir"),
336+
"qdb-slot-lock-delegated-" + System.nanoTime()).toString();
337+
Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT));
338+
String slot = tmpDir + "/slot";
339+
long segSize = MmapSegment.HEADER_SIZE + MmapSegment.FRAME_HEADER_SIZE + 32L;
340+
final CountDownLatch enteredConnect = new CountDownLatch(1);
341+
final CountDownLatch releaseConnect = new CountDownLatch(1);
342+
final AtomicReference<Thread> ioThreadRef = new AtomicReference<>();
343+
final StubWebSocketClient stubClient = new StubWebSocketClient();
344+
// Healthy owned manager: once the wedged I/O thread is released,
345+
// its exit-path engine.close() completes normally and releases
346+
// the flock — the flip this test pins.
347+
CursorSendEngine engine = new CursorSendEngine(slot, segSize);
348+
CursorWebSocketSendLoop loop = null;
349+
QwpWebSocketSender wss = null;
350+
try {
351+
// Stand-in for a blocking native connect(2): entered by the
352+
// loop's I/O thread, immune to unpark, never interrupted by
353+
// loop.close().
354+
CursorWebSocketSendLoop.ReconnectFactory stuckConnect = () -> {
355+
ioThreadRef.set(Thread.currentThread());
356+
enteredConnect.countDown();
357+
releaseConnect.await();
358+
return stubClient;
359+
};
360+
loop = new CursorWebSocketSendLoop(
361+
null /* async-initial-connect: the I/O thread drives the connect */,
362+
engine, 0L, 1_000L,
363+
stuckConnect,
364+
5_000L, 100L, 5_000L, false);
365+
loop.start();
366+
Assert.assertTrue("I/O thread never reached the connect factory",
367+
enteredConnect.await(5, TimeUnit.SECONDS));
368+
369+
wss = QwpWebSocketSender.createForTesting("localhost", 1);
370+
wss.setCursorEngine(engine, true);
371+
setField(wss, "cursorSendLoop", loop);
372+
373+
// Drive the real early-bail close() on a thread whose pending
374+
// interrupt lands in loop.close()'s shutdownLatch.await().
375+
AtomicReference<Throwable> closeFailure = new AtomicReference<>();
376+
QwpWebSocketSender wssRef = wss;
377+
Thread closer = new Thread(() -> {
378+
Thread.currentThread().interrupt();
379+
try {
380+
wssRef.close();
381+
} catch (Throwable t) {
382+
closeFailure.set(t);
383+
}
384+
}, "delegated-close-closer");
385+
closer.setDaemon(true);
386+
closer.start();
387+
closer.join(TimeUnit.SECONDS.toMillis(10));
388+
Assert.assertFalse("closer thread did not finish", closer.isAlive());
389+
Assert.assertNotNull("close() must surface the failed I/O-thread stop",
390+
closeFailure.get());
391+
392+
// The I/O thread is still wedged: the flock is retained and
393+
// must be reported retained.
394+
Assert.assertFalse(
395+
"isSlotLockReleased() must be false while the delegated engine "
396+
+ "close is pending on the wedged I/O thread",
397+
wss.isSlotLockReleased());
398+
Assert.assertFalse("engine close must not have run yet",
399+
engine.isCloseCompleted());
400+
try {
401+
SlotLock probe = SlotLock.acquire(slot);
402+
probe.close();
403+
Assert.fail("slot became acquirable while the delegated engine close "
404+
+ "was still pending");
405+
} catch (Exception expected) {
406+
// good — the flock is genuinely held.
407+
}
408+
409+
// Un-wedge the connect. The I/O thread exits; its exit path
410+
// runs the delegated engine.close(), which releases the flock.
411+
releaseConnect.countDown();
412+
Thread ioThread = ioThreadRef.get();
413+
Assert.assertNotNull(ioThread);
414+
ioThread.join(TimeUnit.SECONDS.toMillis(10));
415+
Assert.assertFalse("I/O thread did not exit after the connect returned",
416+
ioThread.isAlive());
417+
418+
// The recovery contract under test: the getter re-probes the
419+
// retained engine, so the late release MUST become visible —
420+
// this is what lets SenderPool recover the retired slot.
421+
Assert.assertTrue("delegated engine close must have completed on the "
422+
+ "I/O thread's exit path",
423+
engine.isCloseCompleted());
424+
Assert.assertTrue(
425+
"isSlotLockReleased() must flip true once the delegated engine "
426+
+ "close released the flock — otherwise the pool retires the "
427+
+ "slot's capacity until process exit",
428+
wss.isSlotLockReleased());
429+
try (SlotLock probe = SlotLock.acquire(slot)) {
430+
Assert.assertNotNull("slot must be acquirable after the delegated close", probe);
431+
}
432+
} finally {
433+
releaseConnect.countDown();
434+
// Reap the loop's bookkeeping now that the I/O thread is gone
435+
// (close() threw mid-teardown, so ioThread was left set).
436+
Thread.interrupted();
437+
if (loop != null) {
438+
try {
439+
loop.close();
440+
} catch (Throwable ignored) {
441+
}
442+
}
443+
if (engine != null && !engine.isCloseCompleted()) {
444+
try {
445+
engine.close();
446+
} catch (Throwable ignored) {
447+
}
448+
}
449+
// The early-return close() deliberately leaked the resources
450+
// the (then-running) I/O thread might touch; free the same set
451+
// the post-guard tail would have freed.
452+
if (wss != null) {
453+
freeFieldQuietly(wss, "buffer0");
454+
freeFieldQuietly(wss, "buffer1");
455+
freeFieldQuietly(wss, "client");
456+
freeFieldQuietly(wss, "errorDispatcher");
457+
freeFieldQuietly(wss, "progressDispatcher");
458+
freeFieldQuietly(wss, "connectionDispatcher");
459+
}
460+
stubClient.close();
461+
rmDirRecursive(tmpDir);
462+
Files.remove(tmpDir);
463+
}
464+
});
465+
}
466+
308467
// ------------------------------------------------------------------ utils
309468

310469
private static void freeFieldQuietly(Object target, String name) {
@@ -370,6 +529,28 @@ private static void setField(Object target, String name, Object value) throws Ex
370529
throw new NoSuchFieldException(name);
371530
}
372531

532+
/**
533+
* Minimal concrete {@link WebSocketClient} — never performs I/O. Handed
534+
* to the loop by the stuck-connect factory; the loop's exit path closes
535+
* it (close is idempotent via the superclass).
536+
*/
537+
private static final class StubWebSocketClient extends WebSocketClient {
538+
539+
StubWebSocketClient() {
540+
super(DefaultHttpClientConfiguration.INSTANCE, PlainSocketFactory.INSTANCE);
541+
}
542+
543+
@Override
544+
protected void ioWait(int timeout, int op) {
545+
throw new UnsupportedOperationException("stub: no socket");
546+
}
547+
548+
@Override
549+
protected void setupIoWait() {
550+
// no-op
551+
}
552+
}
553+
373554
/** ACKs every binary frame with a running sequence so flush/close drain cleanly. */
374555
private static final class AckAllHandler implements TestWebSocketServer.WebSocketServerHandler {
375556
private final AtomicLong nextSeq = new AtomicLong();

0 commit comments

Comments
 (0)