From f9fd0e86b09f119ce5e52f2f3148806e35f79309 Mon Sep 17 00:00:00 2001
From: Daniel Albers <daniel@lbe.rs>
Date: Fri, 10 Apr 2026 10:04:06 +0200
Subject: [PATCH] fix: Improve SecureSocket handshake and event handling

This fix addresses a busy-loop and stalled connection issue during the TLS
handshake. Previously, SecureSocket incorrectly managed its readiness flags
during negotiation, often polling for 'writability' when OpenSSL was actually
waiting for more 'read' data (or vice versa).

- Explicitly maps SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE to
  socket multiplexer events.
- Resets readiness flags before each handshake attempt to ensure the
  multiplexer receives the most accurate requirements from OpenSSL.
- Restores readability/writability flags upon a successful handshake
  to allow subsequent application-layer protocol exchange.
- Removes inefficient busy-wait sleeps that were masking the loop
  and slowing down handshakes.
---
 src/lib/net/SecureSocket.cpp | 40 +++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/lib/net/SecureSocket.cpp b/src/lib/net/SecureSocket.cpp
index 3198f9690..cef943d34 100644
--- a/src/lib/net/SecureSocket.cpp
+++ b/src/lib/net/SecureSocket.cpp
@@ -85,8 +85,15 @@ void SecureSocket::connect(const NetworkAddress &addr)
 ISocketMultiplexerJob *SecureSocket::newJob()
 {
   // after TCP connection is established, SecureSocket will pick up
-  // connected event and do secureConnect
+  // connected event and do secureConnect. However, we must ensure
+  // that the multiplexer has a job to actually detect that connection.
   if (isConnected() && !m_secureReady) {
+    if (m_ssl && m_ssl->m_ssl) {
+      // we are in the middle of a secure handshake
+      return new TSocketMultiplexerMethodJob<SecureSocket>(
+          this, &SecureSocket::serviceConnect, getSocket(), isReadable(), isWritable()
+      );
+    }
     return nullptr;
   }
 
@@ -443,7 +450,6 @@ int SecureSocket::secureAccept(int socket)
   if (retry > 0) {
     LOG_DEBUG2("retry accepting secure socket");
     m_secureReady = false;
-    Arch::sleep(s_retryDelay);
     return 0;
   }
 
@@ -488,7 +494,6 @@ int SecureSocket::secureConnect(int socket)
   if (retry > 0) {
     LOG_DEBUG2("retry connect secure socket");
     m_secureReady = false;
-    Arch::sleep(s_retryDelay);
     return 0;
   }
 
@@ -549,14 +554,14 @@ void SecureSocket::checkResult(int status, int &retry)
     break;
 
   case SSL_ERROR_WANT_READ:
+    setReadable(true);
     retry++;
     LOG_DEBUG2("want to read, error=%d, attempt=%d", errorCode, retry);
     break;
 
   case SSL_ERROR_WANT_WRITE:
     // Need to make sure the socket is known to be writable so the impending
-    // select action actually triggers on a write. This isn't necessary for
-    // m_readable because the socket logic is always readable
+    // poll action actually triggers on a write.
     setWritable(true);
     retry++;
     LOG_DEBUG2("want to write, error=%d, attempt=%d", errorCode, retry);
@@ -659,6 +664,10 @@ ISocketMultiplexerJob *SecureSocket::serviceConnect(ISocketMultiplexerJob *const
 {
   Lock lock(&getMutex());
 
+  // reset flags so checkResult can set them based on what SSL needs
+  setReadable(false);
+  setWritable(false);
+
   int status = 0;
 #if defined(Q_OS_WIN)
   status = secureConnect(static_cast<int>(getSocket()->m_socket));
@@ -673,11 +682,18 @@ ISocketMultiplexerJob *SecureSocket::serviceConnect(ISocketMultiplexerJob *const
 
   // If status > 0, success
   if (status > 0) {
+    setReadable(true);
+    setWritable(true);
     sendEvent(EventTypes::DataSocketSecureConnected);
     return newJob();
   }
 
-  // Retry case
+  // Retry case. Ensure we poll for what we need.
+  // If checkResult didn't set anything, default to what we were doing.
+  if (!isReadable() && !isWritable()) {
+    setWritable(true);
+  }
+
   return new TSocketMultiplexerMethodJob<SecureSocket>(
       this, &SecureSocket::serviceConnect, getSocket(), isReadable(), isWritable()
   );
@@ -687,6 +703,10 @@ ISocketMultiplexerJob *SecureSocket::serviceAccept(ISocketMultiplexerJob *const,
 {
   Lock lock(&getMutex());
 
+  // reset flags so checkResult can set them based on what SSL needs
+  setReadable(false);
+  setWritable(false);
+
   int status = 0;
 #if defined(Q_OS_WIN)
   status = secureAccept(static_cast<int>(getSocket()->m_socket));
@@ -701,11 +721,17 @@ ISocketMultiplexerJob *SecureSocket::serviceAccept(ISocketMultiplexerJob *const,
 
   // If status > 0, success
   if (status > 0) {
+    setReadable(true);
+    setWritable(true);
     sendEvent(EventTypes::ClientListenerAccepted);
     return newJob();
   }
 
-  // Retry case
+  // Retry case. Ensure we poll for what we need.
+  if (!isReadable() && !isWritable()) {
+    setReadable(true);
+  }
+
   return new TSocketMultiplexerMethodJob<SecureSocket>(
       this, &SecureSocket::serviceAccept, getSocket(), isReadable(), isWritable()
   );