From 766e3ae976bf2e1011a5a96e6a3b8e12474643b7 Mon Sep 17 00:00:00 2001
From: Martin Burian <martin.burian1@bringauto.com>
Date: Tue, 25 Aug 2026 14:26:49 +0200
Subject: [PATCH] fix(operator-stream): recover the single-operator slot from a
 dead peer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

QuicOperatorServer::onConnected() unconditionally overwrote
operatorConnection_, relying entirely on ba-quic-lib's maxConnections to
keep out a second operator. That is fail-open when two handshakes reach
CONNECTED simultaneously, and gave no path to reclaim the slot from a
peer that vanished without a clean QUIC close (kill -9, crash, power
loss) — a hard-killed GUI held streaming-control for the whole car until
the ES process restarted.

Port the fix already shipped in teleop-module's QuicOperatorServer:
onConnected() now does an explicit check-and-set under operatorMutex_
and disconnects a second connection instead of silently replacing the
tracked one. buildSettings() sets disconnectTimeoutMs explicitly to
match ba-quic-lib's own default, mirroring the same precedent.

BAF-1900
---
 .../operator_stream/QuicOperatorServer.hpp    |  6 +++++
 .../QuicOperatorServer.cpp                    | 23 ++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/bringauto/transparent_module_utils/operator_stream/QuicOperatorServer.hpp b/include/bringauto/transparent_module_utils/operator_stream/QuicOperatorServer.hpp
index ae9232d..0da42ef 100644
--- a/include/bringauto/transparent_module_utils/operator_stream/QuicOperatorServer.hpp
+++ b/include/bringauto/transparent_module_utils/operator_stream/QuicOperatorServer.hpp
@@ -52,6 +52,12 @@ struct QuicOperatorServerConfig {
  * Single operator: `quicServer_.maxConnections = 1` rejects a second concurrent connect attempt at
  * the transport layer already (ConnectionShutdown), so — unlike the old hand-rolled version — this
  * class no longer needs its own compare-and-swap "already have an operator" logic.
+ *
+ * BAF-1900 update: the above does not hold for two handshakes reaching CONNECTED simultaneously —
+ * ba-quic-lib's own `QuicServer::onConnected` documents this race as fail-open. `onConnected()`
+ * does guard `operatorConnection_` with an explicit check-and-set after all, refusing
+ * (`disconnect()`) any additional operator once one is set (mirrors teleop-module's
+ * `QuicOperatorServer`).
  */
 class QuicOperatorServer {
 public:
diff --git a/source/transparent_operator_stream/QuicOperatorServer.cpp b/source/transparent_operator_stream/QuicOperatorServer.cpp
index c741408..46332f7 100644
--- a/source/transparent_operator_stream/QuicOperatorServer.cpp
+++ b/source/transparent_operator_stream/QuicOperatorServer.cpp
@@ -31,6 +31,11 @@ QuicOperatorServer::QuicOperatorServer(QuicOperatorServerConfig config, Operator
 	// against quic-lib's QuicServer.cpp) -- so onConnected() below is never called for a connection
 	// that loses the race, and this class no longer needs its own compare-and-swap "already have an
 	// operator" logic.
+	// BAF-1900 update: the above does not hold for two handshakes reaching CONNECTED simultaneously
+	// -- ba-quic-lib's own QuicServer::onConnected documents this race as fail-open. onConnected()
+	// below therefore does guard operatorConnection_ with an explicit check-and-set after all,
+	// refusing (disconnect()) any additional operator once one is set (ported from teleop-module's
+	// QuicOperatorServer, same fix).
 	quicServer_->maxConnections = 1;
 }
 
@@ -57,6 +62,9 @@ bringauto::quic::QuicSettings QuicOperatorServer::buildSettings() const {
 	// (1024) already matches the old PeerUnidiStreamCount setting, so it is left unset here.
 	settings.idleTimeoutMs = 30000;
 	settings.keepAliveIntervalMs = 5000;
+	// Left at ba-quic-lib's own default (5000ms) -- set explicitly so the choice reads as
+	// deliberate, not inherited (mirrors teleop-module's QuicOperatorServer).
+	settings.disconnectTimeoutMs = 5000;
 	settings.sendBufferingEnabled = true;
 	// 2 == QUIC_SERVER_RESUME_AND_ZERORTT (msquic.h). Spelled out as a literal rather than pulling in
 	// <msquic.h> for one enum value that ba-quic-lib doesn't itself expose — ba-quic-lib links
@@ -125,15 +133,24 @@ QuicOperatorServer::SendResult QuicOperatorServer::sendStatus(std::uint32_t modu
 }
 
 void QuicOperatorServer::onConnected(ConnectionId id) {
-	std::lock_guard<std::mutex> lock(operatorMutex_);
-	operatorConnection_ = std::move(id);
-	logInfo("QUIC operator server: operator connected");
+	{
+		std::lock_guard<std::mutex> lock(operatorMutex_);
+		if (!operatorConnection_.has_value()) {
+			operatorConnection_ = std::move(id);
+			logInfo("QUIC operator server: operator connected");
+			return;
+		}
+	}
+	logWarning("QUIC operator server: an operator is already connected, rejecting new one "
+			   "(single-operator)");
+	quicServer_->disconnect(id);
 }
 
 void QuicOperatorServer::onDisconnected(ConnectionId id) {
 	{
 		std::lock_guard<std::mutex> lock(operatorMutex_);
 		if (operatorConnection_ != id) {
+			// BAF-1900: also covers a connection rejected by onConnected()'s own disconnect() call.
 			return; // a connection rejected by maxConnections never became "the operator"
 		}
 		operatorConnection_.reset();