Skip to content

Commit da21b2e

Browse files
committed
Prevent use-after-free when IO thread doesn't stop
While closing the sender, we send the IO thread the stop signal and wait for it to complete. If it doesn't complete on time, we can't just continue with the teardown, freeing the resources it's still using. We must bail out of teardown.
1 parent e63cac9 commit da21b2e

2 files changed

Lines changed: 28 additions & 9 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpWebSocketSender.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import io.questdb.client.std.Decimal256;
4040
import io.questdb.client.std.Decimal64;
4141
import io.questdb.client.std.LongHashSet;
42+
import io.questdb.client.std.Misc;
4243
import io.questdb.client.std.ObjList;
4344
import io.questdb.client.std.bytes.DirectByteSlice;
4445
import org.jetbrains.annotations.NotNull;
@@ -403,6 +404,7 @@ public QwpWebSocketSender charColumn(CharSequence columnName, char value) {
403404
public void close() {
404405
if (!closed) {
405406
closed = true;
407+
boolean ioThreadStopped = true;
406408

407409
// Flush any remaining data
408410
try {
@@ -436,10 +438,16 @@ public void close() {
436438
try {
437439
sendQueue.close();
438440
} catch (Exception e) {
441+
ioThreadStopped = false;
439442
LOG.error("Error closing send queue: {}", String.valueOf(e));
440443
}
441444
}
442445

446+
if (!ioThreadStopped) {
447+
LOG.error("Skipping WebSocket client teardown because the I/O thread is still running");
448+
return;
449+
}
450+
443451
// Close buffers (async mode only, window > 1)
444452
if (buffer0 != null) {
445453
buffer0.close();
@@ -458,10 +466,7 @@ public void close() {
458466
for (int i = 0, n = keys.size(); i < n; i++) {
459467
CharSequence key = keys.getQuick(i);
460468
if (key != null) {
461-
QwpTableBuffer tb = tableBuffers.get(key);
462-
if (tb != null) {
463-
tb.close();
464-
}
469+
Misc.free(tableBuffers.get(key));
465470
}
466471
}
467472
tableBuffers.clear();

core/src/main/java/io/questdb/client/cutlass/qwp/client/WebSocketSendQueue.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,16 @@ public void close() {
205205
}
206206
ioThread.interrupt();
207207

208-
// Wait for I/O thread to finish
209-
try {
210-
shutdownLatch.await(shutdownTimeoutMs, TimeUnit.MILLISECONDS);
211-
} catch (InterruptedException e) {
212-
Thread.currentThread().interrupt();
208+
// Wait for I/O thread to finish before allowing the caller to free
209+
// the socket and client-owned native buffers. If a send/recv call is
210+
// still blocked, disconnect the socket to force it to unwind.
211+
if (!awaitShutdown(shutdownTimeoutMs)) {
212+
LOG.warn("I/O thread did not stop within {}ms, disconnecting socket", shutdownTimeoutMs);
213+
client.disconnect();
214+
ioThread.interrupt();
215+
if (!awaitShutdown(shutdownTimeoutMs)) {
216+
throw new LineSenderException("Timed out waiting for WebSocket I/O thread to stop");
217+
}
213218
}
214219

215220
LOG.info("WebSocket send queue closed [totalBatches={}, totalBytes={}]", totalBatchesSent.get(), totalBytesSent.get());
@@ -503,6 +508,15 @@ private boolean isPendingEmpty() {
503508
return pendingBuffer == null;
504509
}
505510

511+
private boolean awaitShutdown(long timeoutMs) {
512+
try {
513+
return shutdownLatch.await(timeoutMs, TimeUnit.MILLISECONDS);
514+
} catch (InterruptedException e) {
515+
Thread.currentThread().interrupt();
516+
return shutdownLatch.getCount() == 0;
517+
}
518+
}
519+
506520
private boolean offerPending(MicrobatchBuffer buffer) {
507521
if (pendingBuffer != null) {
508522
return false; // slot occupied

0 commit comments

Comments
 (0)