From 9d23c2a7eeed3b4b2d69f6f3aa29547da3aded70 Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Thu, 27 Aug 2026 16:12:02 +0200 Subject: [PATCH] Pre-flight: always answer, and record the invariant the check rests on ProcessPreflight caught JFJochException only. The pusher is blocked on this ACK and reads its absence as a dead writer, so anything else escaping to Run()'s generic handler cost the caller the real reason and handed it a five-second timeout instead - for a check whose entire purpose is to report the problem before the detector is armed. std::filesystem::exists is the concrete leak: it throws filesystem_error, not JFJochException, when the output path cannot be walked at all (a directory component with no search permission, a symlink loop), which is exactly the misconfiguration worth catching early. Now catches std::exception, as JFJochReceiverService::Start does. Also corrects the comment above the write_master_file assignment in TCPStreamPusher::Preflight, which claimed index 0 had to fall on the same connection here as in StartDataCollection. It does not, and cannot be made to: Preflight filters the pool on connected && !broken, while START re-derives its index 0 after DetachDeadConnections, which additionally drops anything failing IsConnectionAlive - so a writer lost in between shifts it. That is harmless, because every writer of a run works on the same filesystem and so answers the same question about the same files. The comment now says that instead. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016L1qig74oYQzfUJJZbbxFh --- image_pusher/TCPStreamPusher.cpp | 8 +++++--- writer/StreamWriter.cpp | 8 +++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/image_pusher/TCPStreamPusher.cpp b/image_pusher/TCPStreamPusher.cpp index 8d2fe829..2fd529ff 100644 --- a/image_pusher/TCPStreamPusher.cpp +++ b/image_pusher/TCPStreamPusher.cpp @@ -866,9 +866,11 @@ void TCPStreamPusher::Preflight(StartMessage& message) { for (size_t i = 0; i < local_connections.size(); i++) { auto& c = *local_connections[i]; - // Same assignment StartDataCollection makes, and it has to be the same one: only the - // writer holding the master file checks the output files, so a different index 0 here - // than there would check the run on the wrong writer. + // The same assignment StartDataCollection makes, so exactly one writer runs the check. It + // does NOT have to fall on the same connection there as here - the pool can lose a writer in + // between, and START re-derives index 0 after detaching the dead ones. Every writer of a run + // works on the same filesystem, so any of them answers the same question about the same + // files; which one is asked does not change the answer. message.socket_number = static_cast(c.socket_number); message.write_master_file = (i == 0); diff --git a/writer/StreamWriter.cpp b/writer/StreamWriter.cpp index 7303ec19..806a8662 100644 --- a/writer/StreamWriter.cpp +++ b/writer/StreamWriter.cpp @@ -99,10 +99,16 @@ void StreamWriter::ProcessPreflight() { run_number = msg.run_number; socket_number = msg.socket_number.value_or(0); + // std::exception, not JFJochException: the answer matters more than the diagnosis. The pusher is + // blocked on this ACK and reads its absence as a dead writer, so anything that escapes here costs + // the caller the real reason and hands it a five-second timeout instead. std::filesystem::exists + // is the concrete leak - it throws filesystem_error, not JFJochException, when the output path + // cannot be walked at all (a directory component with no search permission, a symlink loop), + // which is exactly the kind of misconfiguration the pre-flight exists to report before arming. try { FileWriter::Preflight(msg); NotifyTcpAck(TCPFrameType::PREFLIGHT, true, false, TCPAckCode::None); - } catch (const JFJochException &e) { + } catch (const std::exception &e) { logger.Warning("Pre-flight check failed: {}", e.what()); // Not fatal: nothing was started, so the connection stays usable for the next attempt. NotifyTcpAck(TCPFrameType::PREFLIGHT, false, false, TCPAckCode::StartFailed, e.what());