Skip to content

Commit c7419f4

Browse files
committed
test(qwp): red-first -- interrupted drainer teardown must not release the slot under a live I/O thread (C5 SEGV)
Pins the teardown-overlap invariant: at BackgroundDrainer.run() return, NOT (loop I/O thread alive AND slot lock released). The slot lock is the public witness of engine teardown -- engine.close() releases it strictly after munmap/Unsafe.free -- so a released lock under a live thread means segment memory was freed while raw Unsafe readers could still touch it. The tail additionally requires the lock be EVENTUALLY released once the stuck connect resolves: a fix may defer teardown, never abandon it. Proxy disclosure (fix-shape coupling): the true property, no engine access after unmap, is not assertable in-process (a SEGV kills the JVM; freed malloc memory is not guaranteed to fault). The invariant is a sufficient discipline, deliberately stricter than the minimal property. Red on current HEAD: shutdownNow's interrupt is swallowed by loop.close(), run()'s finally unmaps under the gated I/O thread, and the lock probe succeeds while the thread is alive.
1 parent 2e0719d commit c7419f4

1 file changed

Lines changed: 287 additions & 0 deletions

File tree

Lines changed: 287 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,287 @@
1+
/*+*****************************************************************************
2+
* ___ _ ____ ____
3+
* / _ \ _ _ ___ ___| |_| _ \| __ )
4+
* | | | | | | |/ _ \/ __| __| | | | _ \
5+
* | |_| | |_| | __/\__ \ |_| |_| | |_) |
6+
* \__\_\\__,_|\___||___/\__|____/|____/
7+
*
8+
* Copyright (c) 2014-2019 Appsicle
9+
* Copyright (c) 2019-2026 QuestDB
10+
*
11+
* Licensed under the Apache License, Version 2.0 (the "License");
12+
* you may not use this file except in compliance with the License.
13+
* You may obtain a copy of the License at
14+
*
15+
* http://www.apache.org/licenses/LICENSE-2.0
16+
*
17+
* Unless required by applicable law or agreed to in writing, software
18+
* distributed under the License is distributed on an "AS IS" BASIS,
19+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
20+
* See the License for the specific language governing permissions and
21+
* limitations under the License.
22+
*
23+
******************************************************************************/
24+
25+
package io.questdb.client.test.cutlass.qwp.client.sf.cursor;
26+
27+
import io.questdb.client.DefaultHttpClientConfiguration;
28+
import io.questdb.client.cutlass.http.client.WebSocketClient;
29+
import io.questdb.client.cutlass.qwp.client.sf.cursor.BackgroundDrainer;
30+
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorSendEngine;
31+
import io.questdb.client.cutlass.qwp.client.sf.cursor.CursorWebSocketSendLoop;
32+
import io.questdb.client.network.PlainSocketFactory;
33+
import io.questdb.client.std.Compat;
34+
import io.questdb.client.std.Files;
35+
import io.questdb.client.std.MemoryTag;
36+
import io.questdb.client.std.Unsafe;
37+
import io.questdb.client.test.tools.TestUtils;
38+
import org.junit.After;
39+
import org.junit.Assert;
40+
import org.junit.Before;
41+
import org.junit.Test;
42+
43+
import java.nio.file.Paths;
44+
import java.util.concurrent.CountDownLatch;
45+
import java.util.concurrent.TimeUnit;
46+
import java.util.concurrent.atomic.AtomicInteger;
47+
import java.util.concurrent.atomic.AtomicReference;
48+
49+
/**
50+
* Red test for the SEGV half of finding C5 — interrupted drainer teardown
51+
* must not unmap the engine under a live I/O thread.
52+
* <p>
53+
* Production sequence: during an outage the drainer's loop I/O thread sits
54+
* inside a blocking native connect ({@code connect_timeout} defaults to 0 =
55+
* OS timeout; neither unpark nor interrupt cancels {@code connect(2)}).
56+
* {@code BackgroundDrainerPool.close()} escalates — graceful drain,
57+
* {@code requestStop()}, then {@code shutdownNow()} — and the interrupt
58+
* lands in {@code loop.close()}'s {@code shutdownLatch.await()}. Pre-fix,
59+
* {@code close()} swallows it and returns while the I/O thread is alive;
60+
* {@code BackgroundDrainer.run()}'s finally then closes the engine —
61+
* {@code munmap}/{@code Unsafe.free} on segment memory a thread that is
62+
* still alive may touch with raw {@code Unsafe} reads.
63+
* <p>
64+
* The invariant pinned here: <b>at the moment {@code run()} returns, NOT
65+
* (loop I/O thread alive AND slot lock released)</b>. The slot lock is an
66+
* on-disk protocol shared with other processes and scanners, and
67+
* {@code engine.close()} releases it strictly after unmapping — so
68+
* "lock released" is the public, behavioral witness of "engine torn down".
69+
* Either valid fix shape satisfies the invariant: block until the thread
70+
* exits (re-await), or keep the lock/engine alive past {@code run()} by
71+
* delegating engine teardown to the I/O thread's exit path. The tail of the
72+
* test additionally requires that the slot lock is EVENTUALLY released once
73+
* the stuck connect resolves — a fix may defer teardown, not abandon it.
74+
* <p>
75+
* NOTE: this test is a proxy for the memory-safety property ("no engine
76+
* access after unmap"), which cannot be asserted in-process — a SEGV kills
77+
* the JVM, and {@code Unsafe.free}'d memory is not guaranteed to fault. The
78+
* invariant is a sufficient teardown discipline, deliberately stricter than
79+
* the minimal property; see the C5 review discussion.
80+
* <p>
81+
* Determinism: no sleeps. The interrupt is delivered after
82+
* {@code requestStop()} while the runner is either in an interrupt-immune
83+
* park ({@code LockSupport.parkNanos} preserves the flag) or already in the
84+
* latch await — both routes arrive at the await with the flag set, which
85+
* throws before parking. The "stuck connect" is a latch-gated factory
86+
* (unpark-immune; {@code close()} never interrupts the I/O thread). The
87+
* test only runs safely on pre-fix code because the already-landed
88+
* discard-when-stopped fix keeps the post-teardown I/O thread away from
89+
* engine memory — the hazard this test guards is real on any HEAD without
90+
* that commit.
91+
*/
92+
public class BackgroundDrainerInterruptedTeardownTest {
93+
94+
private static final long SEGMENT_BYTES = 64 * 1024;
95+
private String tmpDir;
96+
97+
@Before
98+
public void setUp() {
99+
tmpDir = Paths.get(System.getProperty("java.io.tmpdir"),
100+
"qdb-c5-teardown-" + System.nanoTime()).toString();
101+
Assert.assertEquals(0, Files.mkdir(tmpDir, Files.DIR_MODE_DEFAULT));
102+
}
103+
104+
@After
105+
public void tearDown() {
106+
if (tmpDir == null) return;
107+
long find = Files.findFirst(tmpDir);
108+
if (find > 0) {
109+
try {
110+
int rc = 1;
111+
while (rc > 0) {
112+
String name = Files.utf8ToString(Files.findName(find));
113+
if (name != null && !".".equals(name) && !"..".equals(name)) {
114+
Files.remove(tmpDir + "/" + name);
115+
}
116+
rc = Files.findNext(find);
117+
}
118+
} finally {
119+
Files.findClose(find);
120+
}
121+
}
122+
Files.remove(tmpDir);
123+
}
124+
125+
@Test
126+
public void testC5_interruptedTeardownMustNotReleaseSlotUnderLiveIoThread() throws Exception {
127+
TestUtils.assertMemoryLeak(() -> {
128+
// 1. Slot with one published, unacked frame so the drainer opens
129+
// a real engine and spins up a send loop.
130+
long buf = Unsafe.malloc(16, MemoryTag.NATIVE_DEFAULT);
131+
try {
132+
CursorSendEngine prep = new CursorSendEngine(tmpDir, SEGMENT_BYTES);
133+
try {
134+
for (int i = 0; i < 16; i++) {
135+
Unsafe.getUnsafe().putByte(buf + i, (byte) i);
136+
}
137+
Assert.assertEquals(0L, prep.appendBlocking(buf, 16));
138+
} finally {
139+
// Unacked data on disk -> close() keeps the .sfa files.
140+
prep.close();
141+
}
142+
} finally {
143+
Unsafe.free(buf, 16, MemoryTag.NATIVE_DEFAULT);
144+
}
145+
146+
final CountDownLatch enteredReconnect = new CountDownLatch(1);
147+
final CountDownLatch releaseConnect = new CountDownLatch(1);
148+
final AtomicInteger connects = new AtomicInteger();
149+
final AtomicReference<Thread> ioThreadRef = new AtomicReference<>();
150+
// Client #1: initial connect succeeds (on the runner thread), but
151+
// every send throws -- driving the I/O thread into its reconnect
152+
// loop. Client #2: handed back by the gated "connect" after the
153+
// teardown has already run.
154+
final StubWebSocketClient wireDownClient = new StubWebSocketClient();
155+
final StubWebSocketClient postTeardownClient = new StubWebSocketClient();
156+
157+
final CursorWebSocketSendLoop.ReconnectFactory factory = () -> {
158+
if (connects.incrementAndGet() == 1) {
159+
// Initial connect: runs on the drainer's runner thread.
160+
return wireDownClient;
161+
}
162+
// Wire-failure reconnect: runs on the loop's I/O thread.
163+
// Stand-in for a blocking native connect(2): unpark-immune
164+
// (a latch await re-parks after a spurious wake) and never
165+
// interrupted (loop.close() only unparks).
166+
ioThreadRef.set(Thread.currentThread());
167+
enteredReconnect.countDown();
168+
releaseConnect.await();
169+
return postTeardownClient;
170+
};
171+
172+
final BackgroundDrainer drainer = new BackgroundDrainer(
173+
tmpDir, SEGMENT_BYTES, Long.MAX_VALUE, factory,
174+
5_000L, 10L, 50L, false, 0L);
175+
176+
Thread runner = new Thread(drainer::run, "drainer-runner");
177+
runner.setDaemon(true);
178+
runner.start();
179+
try {
180+
Assert.assertTrue("I/O thread never reached the reconnect factory",
181+
enteredReconnect.await(10, TimeUnit.SECONDS));
182+
183+
// Pool-shutdown stand-in: requestStop, then the shutdownNow
184+
// interrupt. Wherever the runner is at this instant -- the
185+
// poll park (flag-preserving) or already in the latch await --
186+
// the flag arrives at the await and throws before parking.
187+
drainer.requestStop();
188+
runner.interrupt();
189+
runner.join(10_000L);
190+
Assert.assertFalse("drainer did not return after stop + interrupt",
191+
runner.isAlive());
192+
Assert.assertEquals(BackgroundDrainer.DrainOutcome.STOPPED,
193+
drainer.outcome());
194+
195+
Thread ioThread = ioThreadRef.get();
196+
Assert.assertNotNull(ioThread);
197+
boolean ioThreadAliveAtReturn = ioThread.isAlive();
198+
boolean slotLockFreeAtReturn = isSlotLockFree();
199+
Assert.assertFalse(
200+
"C5 (SEGV): BackgroundDrainer.run() returned with the slot lock "
201+
+ "released (engine closed -- segments munmap'd/freed) while "
202+
+ "the loop's I/O thread was still alive inside a blocking "
203+
+ "connect. loop.close() swallowed the InterruptedException "
204+
+ "from shutdownLatch.await() and returned; the finally then "
205+
+ "unmapped memory a live thread may touch with raw Unsafe "
206+
+ "reads. Teardown must either wait for the thread or be "
207+
+ "delegated to its exit path.",
208+
ioThreadAliveAtReturn && slotLockFreeAtReturn);
209+
} finally {
210+
// Unblock the "connect" and quiesce regardless of verdict so
211+
// the memory-leak wrapper sees a fully wound-down world.
212+
releaseConnect.countDown();
213+
Thread ioThread = ioThreadRef.get();
214+
if (ioThread != null) {
215+
ioThread.join(10_000L);
216+
Assert.assertFalse("I/O thread did not exit after the connect returned",
217+
ioThread.isAlive());
218+
}
219+
wireDownClient.close();
220+
postTeardownClient.close();
221+
}
222+
223+
// Deferred is fine; abandoned is not: once the stuck connect
224+
// resolved and the I/O thread exited, the slot lock must be
225+
// released (engine closed by whoever ended up owning teardown),
226+
// or no scanner can ever adopt the slot's remaining data.
227+
long deadlineNanos = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
228+
while (!isSlotLockFree()) {
229+
Assert.assertTrue(
230+
"slot lock never released after the I/O thread exited -- "
231+
+ "engine teardown was abandoned, not deferred",
232+
System.nanoTime() < deadlineNanos);
233+
Compat.onSpinWait();
234+
}
235+
});
236+
}
237+
238+
/**
239+
* Public, behavioral probe of the slot lock: opening an engine on the
240+
* slot succeeds iff no other engine holds the on-disk lock. The probe
241+
* engine is closed immediately; the slot's unacked data keeps its files
242+
* on disk, so probing is observation-only.
243+
*/
244+
private boolean isSlotLockFree() {
245+
try {
246+
new CursorSendEngine(tmpDir, SEGMENT_BYTES).close();
247+
return true;
248+
} catch (IllegalStateException e) {
249+
String msg = e.getMessage();
250+
if (msg != null && msg.contains("already in use")) {
251+
return false;
252+
}
253+
throw e;
254+
}
255+
}
256+
257+
/**
258+
* Minimal concrete {@link WebSocketClient}: connect-level collaborator
259+
* only. Every send throws, so handing it to a live loop deterministically
260+
* drives the I/O thread into its reconnect path without native I/O.
261+
*/
262+
private static final class StubWebSocketClient extends WebSocketClient {
263+
StubWebSocketClient() {
264+
super(DefaultHttpClientConfiguration.INSTANCE, PlainSocketFactory.INSTANCE);
265+
}
266+
267+
@Override
268+
public void sendBinary(long dataPtr, int length) {
269+
throw new IllegalStateException("stub: wire down");
270+
}
271+
272+
@Override
273+
public void sendBinary(long dataPtr, int length, int timeout) {
274+
throw new IllegalStateException("stub: wire down");
275+
}
276+
277+
@Override
278+
protected void ioWait(int timeout, int op) {
279+
throw new UnsupportedOperationException("stub: no socket");
280+
}
281+
282+
@Override
283+
protected void setupIoWait() {
284+
// no-op
285+
}
286+
}
287+
}

0 commit comments

Comments
 (0)