Skip to content

Commit da4bd1d

Browse files
committed
Bound the TLS handshake and drive it off the event loop
JavaTlsClientSocket.startTlsSession ran the TLS handshake with raw delegate.recv/delegate.send on a non-blocking socket and never waited on socket readiness. A peer that completed the TCP connect but stalled before its half of the handshake left the engine in NEED_UNWRAP with recv returning 0 (would-block), so the loop re-read in a tight cycle: a 100% CPU busy-spin with no deadline. connect_timeout bounded only the TCP connect, and the WebSocket upgrade's auth/request timeout never covered the handshake, so a stalled wss:// (e.g. QuestDB Cloud) handshake could pin a core indefinitely and defeat a bounded connect. Drive the handshake through the client's existing deadline-aware ioWait, the same primitive recvOrDie/doSend already use. When the socket would block, the handshake hands control to a SocketReadinessWaiter that parks on epoll/kqueue/select for the remaining connect budget and throws a timeout-flagged exception once it is spent. This removes the spin and bounds the handshake in one change: both NEED_UNWRAP (recv == 0) and the NEED_WRAP send loop (send == 0) now wait instead of spinning. doConnect now calls setupIoWait() before the handshake (so the fd is registered when the waiter parks) and bounds the handshake by connect_timeout, falling back to the request timeout when connect_timeout is unset, so the handshake can no longer hang or spin even with the default config. The connect TLS block disconnects on any handshake error (including the waiter's timeout) so the fd and native buffers do not leak. Both HttpClient (https) and WebSocketClient (wss) connect paths share the fix. Extracted the handshake loop into runHandshake(waiter) so a stub SSLEngine can exercise the wait paths. Added two tests: testHandshakeWaitsForReadabilityInsteadOfBusySpinning drives a stalled peer and asserts the handshake yields to the waiter exactly once (a method-level timeout fails the test if the spin returns), and testHandshakeCompletesWithoutWaitingWhenEngineMakesProgress guards the happy path.
1 parent bcb1e7a commit da4bd1d

10 files changed

Lines changed: 325 additions & 88 deletions

File tree

core/src/main/java/io/questdb/client/cutlass/http/client/HttpClient.java

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,19 +639,36 @@ private void connect(CharSequence host, int port) {
639639
throw new HttpClientException("could not configure socket to be non-blocking [fd=").put(fd).put(", errno=").put(errno).put(']');
640640
}
641641

642+
// Register the fd with the event loop before the TLS handshake so the
643+
// handshake can park on socket readiness via ioWait() instead of
644+
// busy-spinning on the non-blocking socket.
645+
setupIoWait();
646+
642647
if (socket.supportsTls()) {
648+
// Bound the TLS handshake by the connect budget (falling back to
649+
// the request timeout when connect_timeout is unset), so a peer
650+
// that completes TCP but stalls mid-handshake cannot hang or pin a
651+
// CPU.
652+
final long tlsHandshakeStartNanos = System.nanoTime();
653+
final int tlsHandshakeBudgetMillis = connectTimeout > 0 ? connectTimeout : defaultTimeout;
643654
try {
644-
socket.startTlsSession(host);
655+
socket.startTlsSession(host, op -> ioWait(remainingTime(tlsHandshakeBudgetMillis, tlsHandshakeStartNanos), op));
645656
} catch (TlsSessionInitFailedException e) {
646657
int errno = nf.errno();
647658
disconnect();
648659
throw new HttpClientException("could not start TLS session [fd=").put(fd)
649660
.put(", error=").put(e.getFlyweightMessage())
650661
.put(", errno=").put(errno)
651662
.put(']');
663+
} catch (Throwable t) {
664+
// ioWait() throws a timeout-flagged HttpClientException when the
665+
// handshake budget is exhausted; any other error can also surface
666+
// mid-handshake. Disconnect so the fd and native buffers do not
667+
// leak, then propagate.
668+
disconnect();
669+
throw t;
652670
}
653671
}
654-
setupIoWait();
655672
}
656673

657674
private void doSend(long lo, long hi, int timeoutMillis) {

core/src/main/java/io/questdb/client/cutlass/http/client/WebSocketClient.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -962,19 +962,35 @@ private void doConnect(CharSequence host, int port) {
962962
.put(", errno=").put(errno).put(']');
963963
}
964964

965+
// Register the fd with the event loop before the TLS handshake so the
966+
// handshake can park on socket readiness via ioWait() instead of
967+
// busy-spinning on the non-blocking socket.
968+
setupIoWait();
969+
965970
if (socket.supportsTls()) {
971+
// Bound the TLS handshake by the connect budget (falling back to the
972+
// request timeout when connect_timeout is unset), so a peer that
973+
// completes TCP but stalls mid-handshake cannot hang or pin a CPU.
974+
final long tlsHandshakeStartNanos = System.nanoTime();
975+
final int tlsHandshakeBudgetMillis = connectTimeoutMillis > 0 ? connectTimeoutMillis : defaultTimeout;
966976
try {
967-
socket.startTlsSession(host);
977+
socket.startTlsSession(host, op -> ioWait(getRemainingTimeOrThrow(tlsHandshakeBudgetMillis, tlsHandshakeStartNanos), op));
968978
} catch (TlsSessionInitFailedException e) {
969979
int errno = nf.errno();
970980
disconnect();
971981
throw new HttpClientException("could not start TLS session [fd=").put(fd)
972982
.put(", error=").put(e.getFlyweightMessage())
973983
.put(", errno=").put(errno).put(']');
984+
} catch (Throwable t) {
985+
// ioWait() throws a timeout-flagged HttpClientException when the
986+
// handshake budget is exhausted; any other error can also surface
987+
// mid-handshake. Disconnect so the fd and native buffers do not
988+
// leak, then propagate.
989+
disconnect();
990+
throw t;
974991
}
975992
}
976993

977-
setupIoWait();
978994
if (LOG.isDebugEnabled()) {
979995
LOG.debug("Connected to [host={}, port={}]", host, port);
980996
}

core/src/main/java/io/questdb/client/network/JavaTlsClientSocket.java

Lines changed: 104 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -307,91 +307,13 @@ public int send(long bufferPtr, int bufferLen) {
307307
}
308308

309309
@Override
310-
public void startTlsSession(CharSequence peerName) throws TlsSessionInitFailedException {
310+
public void startTlsSession(CharSequence peerName, SocketReadinessWaiter waiter) throws TlsSessionInitFailedException {
311311
assert state == STATE_PLAINTEXT;
312312
prepareInternalBuffers();
313313
try {
314314
this.sslEngine = createSslEngine(peerName);
315315
this.sslEngine.beginHandshake();
316-
SSLEngineResult.HandshakeStatus handshakeStatus = sslEngine.getHandshakeStatus();
317-
while (handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
318-
switch (handshakeStatus) {
319-
case NEED_TASK:
320-
Runnable task;
321-
while ((task = sslEngine.getDelegatedTask()) != null) {
322-
task.run();
323-
}
324-
handshakeStatus = sslEngine.getHandshakeStatus();
325-
break;
326-
case NEED_WRAP: {
327-
SSLEngineResult result = sslEngine.wrap(wrapInputBuffer, wrapOutputBuffer);
328-
handshakeStatus = result.getHandshakeStatus();
329-
switch (result.getStatus()) {
330-
case BUFFER_UNDERFLOW:
331-
// there cannot be underflow since wrap() during handshake does not read from the input buffer at all
332-
throw new AssertionError("Buffer underflow during TLS handshake. This should not happen. please report as a bug");
333-
case BUFFER_OVERFLOW:
334-
if (wrapOutputBuffer.position() != 0) {
335-
// wrap() left bytes behind without producing a complete record. The OK
336-
// branch is the only place that drains and clears, so a non-empty
337-
// buffer here means we would re-enter NEED_WRAP with identical state
338-
// and spin forever. Fail loudly instead.
339-
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
340-
}
341-
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
342-
// but that would indicate our starting buffer is too small.
343-
growWrapOutputBuffer();
344-
break;
345-
case OK:
346-
// wrapOutputBuffer: write mode
347-
int written = 0;
348-
int bufferLimit = wrapOutputBuffer.position();
349-
while (written < bufferLimit) {
350-
int n = delegate.send(wrapOutputBufferPtr + written, bufferLimit - written);
351-
if (n < 0) {
352-
throw TlsSessionInitFailedException.instance("socket write error");
353-
}
354-
written += n;
355-
}
356-
wrapOutputBuffer.clear();
357-
break;
358-
case CLOSED:
359-
throw TlsSessionInitFailedException.instance("server closed connection unexpectedly");
360-
}
361-
break;
362-
}
363-
case NEED_UNWRAP: {
364-
int n = readFromSocket();
365-
if (n < 0) {
366-
throw TlsSessionInitFailedException.instance("socket read error");
367-
}
368-
SSLEngineResult result = sslEngine.unwrap(unwrapInputBuffer, unwrapOutputBuffer);
369-
handshakeStatus = result.getHandshakeStatus();
370-
switch (result.getStatus()) {
371-
case BUFFER_UNDERFLOW:
372-
// we need to receive more data from a socket, let's try again
373-
break;
374-
case BUFFER_OVERFLOW:
375-
if (unwrapOutputBuffer.position() != 0) {
376-
// unwrap() produced plaintext but signalled overflow without consuming
377-
// the next record. Nothing in the handshake loop drains this buffer,
378-
// so re-entering NEED_UNWRAP would spin forever. Fail loudly.
379-
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
380-
}
381-
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
382-
// but that would indicate our starting buffer is too small.
383-
growUnwrapOutputBuffer();
384-
break;
385-
case OK:
386-
// good, let's see what we need to do next
387-
break;
388-
case CLOSED:
389-
throw TlsSessionInitFailedException.instance("server closed connection unexpectedly");
390-
}
391-
}
392-
break;
393-
}
394-
}
316+
runHandshake(waiter);
395317
// unwrap input buffer: read mode and empty
396318
unwrapInputBuffer.position(0);
397319
unwrapInputBuffer.limit(0);
@@ -583,6 +505,108 @@ private int readFromSocket() {
583505
return n;
584506
}
585507

508+
/**
509+
* Drives the TLS handshake state machine to completion. When the
510+
* non-blocking socket would block, hands control to {@code waiter} (which
511+
* parks on the event loop bounded by the connect deadline) instead of
512+
* busy-spinning on read/write. Extracted from {@link #startTlsSession} so a
513+
* stub {@code sslEngine} can exercise the wait paths in isolation.
514+
*/
515+
private void runHandshake(SocketReadinessWaiter waiter) throws SSLException, TlsSessionInitFailedException {
516+
SSLEngineResult.HandshakeStatus handshakeStatus = sslEngine.getHandshakeStatus();
517+
while (handshakeStatus != SSLEngineResult.HandshakeStatus.FINISHED) {
518+
switch (handshakeStatus) {
519+
case NEED_TASK:
520+
Runnable task;
521+
while ((task = sslEngine.getDelegatedTask()) != null) {
522+
task.run();
523+
}
524+
handshakeStatus = sslEngine.getHandshakeStatus();
525+
break;
526+
case NEED_WRAP: {
527+
SSLEngineResult result = sslEngine.wrap(wrapInputBuffer, wrapOutputBuffer);
528+
handshakeStatus = result.getHandshakeStatus();
529+
switch (result.getStatus()) {
530+
case BUFFER_UNDERFLOW:
531+
// there cannot be underflow since wrap() during handshake does not read from the input buffer at all
532+
throw new AssertionError("Buffer underflow during TLS handshake. This should not happen. please report as a bug");
533+
case BUFFER_OVERFLOW:
534+
if (wrapOutputBuffer.position() != 0) {
535+
// wrap() left bytes behind without producing a complete record. The OK
536+
// branch is the only place that drains and clears, so a non-empty
537+
// buffer here means we would re-enter NEED_WRAP with identical state
538+
// and spin forever. Fail loudly instead.
539+
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
540+
}
541+
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
542+
// but that would indicate our starting buffer is too small.
543+
growWrapOutputBuffer();
544+
break;
545+
case OK:
546+
// wrapOutputBuffer: write mode
547+
int written = 0;
548+
int bufferLimit = wrapOutputBuffer.position();
549+
while (written < bufferLimit) {
550+
int n = delegate.send(wrapOutputBufferPtr + written, bufferLimit - written);
551+
if (n < 0) {
552+
throw TlsSessionInitFailedException.instance("socket write error");
553+
}
554+
if (n == 0) {
555+
// The non-blocking socket's send buffer is full. Wait for it to
556+
// become writable -- bounded by the connect deadline -- instead of
557+
// busy-spinning on send().
558+
waiter.awaitReady(IOOperation.WRITE);
559+
}
560+
written += n;
561+
}
562+
wrapOutputBuffer.clear();
563+
break;
564+
case CLOSED:
565+
throw TlsSessionInitFailedException.instance("server closed connection unexpectedly");
566+
}
567+
break;
568+
}
569+
case NEED_UNWRAP: {
570+
int n = readFromSocket();
571+
if (n < 0) {
572+
throw TlsSessionInitFailedException.instance("socket read error");
573+
}
574+
SSLEngineResult result = sslEngine.unwrap(unwrapInputBuffer, unwrapOutputBuffer);
575+
handshakeStatus = result.getHandshakeStatus();
576+
switch (result.getStatus()) {
577+
case BUFFER_UNDERFLOW:
578+
// Not enough bytes for a complete TLS record yet. If the last read
579+
// drained the socket (n == 0, would-block on the non-blocking fd), wait
580+
// for it to become readable -- bounded by the connect deadline -- instead
581+
// of busy-spinning. A positive n means we read a partial record, so loop
582+
// immediately and read the rest.
583+
if (n == 0) {
584+
waiter.awaitReady(IOOperation.READ);
585+
}
586+
break;
587+
case BUFFER_OVERFLOW:
588+
if (unwrapOutputBuffer.position() != 0) {
589+
// unwrap() produced plaintext but signalled overflow without consuming
590+
// the next record. Nothing in the handshake loop drains this buffer,
591+
// so re-entering NEED_UNWRAP would spin forever. Fail loudly.
592+
throw new AssertionError("Buffer overflow during TLS handshake with non-empty output buffer. This should not happen, please report as a bug");
593+
}
594+
// in theory, this can happen if the output buffer is too small to fit a single TLS handshake record,
595+
// but that would indicate our starting buffer is too small.
596+
growUnwrapOutputBuffer();
597+
break;
598+
case OK:
599+
// good, let's see what we need to do next
600+
break;
601+
case CLOSED:
602+
throw TlsSessionInitFailedException.instance("server closed connection unexpectedly");
603+
}
604+
}
605+
break;
606+
}
607+
}
608+
}
609+
586610
private int writeToSocket(int bytesToSend) {
587611
// wrapOutputBuffer is in the write mode
588612
int n = delegate.send(wrapOutputBufferPtr, bytesToSend);

core/src/main/java/io/questdb/client/network/PlainSocket.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public int send(long bufferPtr, int bufferLen) {
7171
}
7272

7373
@Override
74-
public void startTlsSession(CharSequence peerName) {
74+
public void startTlsSession(CharSequence peerName, SocketReadinessWaiter waiter) {
7575
throw new UnsupportedOperationException();
7676
}
7777

core/src/main/java/io/questdb/client/network/Socket.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,12 @@ public interface Socket extends QuietCloseable {
8484
* on server connections.
8585
*
8686
* @param peerName server name to use for SNI and certificate validation.
87+
* @param waiter blocks until the socket is ready for the next handshake
88+
* read/write (bounded by the connect deadline), so the
89+
* handshake does not busy-spin on the non-blocking socket.
8790
* @throws TlsSessionInitFailedException if the call fails.
8891
*/
89-
void startTlsSession(@Nullable CharSequence peerName) throws TlsSessionInitFailedException;
92+
void startTlsSession(@Nullable CharSequence peerName, SocketReadinessWaiter waiter) throws TlsSessionInitFailedException;
9093

9194
/**
9295
* @return true if the socket support TLS encryption; false otherwise.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
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.network;
26+
27+
/**
28+
* Blocks until a non-blocking socket is ready for a given I/O operation, or
29+
* throws a timeout-flagged exception once the caller's deadline passes.
30+
* <p>
31+
* Used to drive the TLS handshake off the client's event loop: instead of
32+
* busy-spinning on a non-blocking socket that returns "would block", the
33+
* handshake hands control to this waiter, which parks on epoll/kqueue/select
34+
* with the remaining connect budget. This bounds the handshake by the same
35+
* deadline as the TCP connect and keeps a stalled peer from pinning a CPU.
36+
*/
37+
@FunctionalInterface
38+
public interface SocketReadinessWaiter {
39+
/**
40+
* Blocks until the socket is ready for {@code ioOperation}, or throws a
41+
* timeout-flagged exception when the connect deadline is exceeded.
42+
*
43+
* @param ioOperation {@link IOOperation#READ} or {@link IOOperation#WRITE}
44+
*/
45+
void awaitReady(int ioOperation);
46+
}
Binary file not shown.

0 commit comments

Comments
 (0)