Fix master hang on slave parse error

When a reservoir coupling slave fails during deck parsing, the
master hangs on MPI_Recv waiting for initial data that never
arrives. Fix by adding a status handshake after MPI_Comm_spawn:
the slave sends OK/FAILED status before any data exchange, and
the master checks it, disconnects, and throws on failure.

Key changes:
- Add SlaveStatus message tag and status handshake protocol
- Slave: in readDeck(), override PARSE_MISSING_INCLUDE to throw
  instead of std::exit(), enabling cooperative MPI shutdown
- Slave: send status to master in sendAndReceiveInitialData()
- Master: receiveSlaveStatus_() checks status, disconnects all
  slave communicators and throws on failure
- Main: notifyMasterSlaveInitFailed_() sends failure status from
  the readDeck() catch handler

opm/simulators/flow/Main.cpp

@@ -279,6 +279,29 @@ void Main::handleTestSplitCommunicatorCmdLine_()
     }
 }
 
+void Main::notifyMasterSlaveInitFailed_()
+{
+#if HAVE_MPI
+    if (Parameters::Get<Parameters::Slave>()) {
+        MPI_Comm parent_comm = MPI_COMM_NULL;
+        MPI_Comm_get_parent(&parent_comm);
+        if (parent_comm != MPI_COMM_NULL) {
+            int rank;
+            MPI_Comm_rank(MPI_COMM_WORLD, &rank);
+            if (rank == 0) {
+                int slave_status = 1;  // FAILED
+                MPI_Send(
+                    &slave_status, 1, MPI_INT, /*dest=*/0,
+                    static_cast<int>(ReservoirCoupling::MessageTag::SlaveStatus),
+                    parent_comm
+                );
+            }
+            MPI_Comm_disconnect(&parent_comm);
+        }
+    }
+#endif
+}
+
 void Main::readDeck(const std::string& deckFilename,
                     const std::string& outputDir,
                     const std::string& outputMode,

opm/simulators/flow/Main.hpp

@@ -42,6 +42,7 @@
 
 #if HAVE_MPI
 #include <opm/simulators/utils/ParallelEclipseState.hpp>
+#include <opm/simulators/flow/rescoup/ReservoirCoupling.hpp>
 #endif
 
 #if HAVE_CUDA
@@ -341,6 +342,12 @@ protected:
                 std::cerr << "Failed to create valid EclipseState object." << std::endl;
                 std::cerr << e.what() << std::endl;
             }
+#if HAVE_MPI
+            // If we are a spawned slave, notify the master that initialization
+            // failed so it does not hang waiting for our initial data exchange.
+            // Only rank 0 sends the status (master receives from source=0).
+            notifyMasterSlaveInitFailed_();
+#endif
             exitCode = EXIT_FAILURE;
             return false;
         }
@@ -389,6 +396,10 @@ private:
     // use the parameter system instead.
     void handleTestSplitCommunicatorCmdLine_();
 
+    // If this process is a reservoir coupling slave, notify the master that
+    // initialization failed so it does not hang waiting for the initial data exchange.
+    void notifyMasterSlaveInitFailed_();
+
     /// Dispatch to actual simulation functions based on input deck's setup.
     ///
     /// Called from runDynamic()

opm/simulators/flow/rescoup/ReservoirCoupling.cpp

@@ -129,43 +129,47 @@ void setErrhandler(MPI_Comm comm, bool is_master)
 // ---------------------------
 
 void Logger::debug(const std::string &msg) const {
-    if (haveDeferredLogger()) {
+    forward_(msg,
-        // DeferredLogger: All ranks log - messages will be gathered later
+        [](DeferredLogger &dl, const std::string &m) { dl.debug(m); },
-        this->deferred_logger_->debug(msg);
+        [](const std::string &m) { OpmLog::debug(m); });
-    } else {
+}
-        // OpmLog fallback: Only rank 0 logs (see comment in info() below)
+
-        if (comm_.rank() == 0) {
+void Logger::error(const std::string &msg) const {
-            OpmLog::debug(msg);
+    forward_(msg,
-        }
+        [](DeferredLogger &dl, const std::string &m) { dl.error(m); },
-    }
+        [](const std::string &m) { OpmLog::error(m); });
 }
 
 void Logger::info(const std::string &msg) const {
+    forward_(msg,
+        [](DeferredLogger &dl, const std::string &m) { dl.info(m); },
+        [](const std::string &m) { OpmLog::info(m); });
+}
+
+void Logger::warning(const std::string &msg) const {
+    forward_(msg,
+        [](DeferredLogger &dl, const std::string &m) { dl.warning(m); },
+        [](const std::string &m) { OpmLog::warning(m); });
+}
+
+// Private methods of Logger
+// -------------------------
+
+// Forward a log message to either DeferredLogger (all ranks) or OpmLog (rank 0 only).
+// DeferredLogger being null is the expected normal state - it's only temporarily set
+// during beginTimeStep() via ScopedLoggerGuard. Messages logged here are identical on
+// all ranks, so rank-0-only OpmLog fallback preserves all information.
+template<typename DeferredFn, typename OpmLogFn>
+void Logger::forward_(const std::string &msg, DeferredFn deferred_fn, OpmLogFn opmlog_fn) const {
     if (haveDeferredLogger()) {
-        // DeferredLogger: All ranks log - messages will be gathered later
+        deferred_fn(*this->deferred_logger_, msg);
-        this->deferred_logger_->info(msg);
     } else {
-        // OpmLog fallback: Only rank 0 logs to avoid logging fallout files.
-        // NOTE: DeferredLogger being null is the expected normal state - it's only
-        // temporarily set during beginTimeStep() via ScopedLoggerGuard. Messages logged
-        // here are identical on all ranks, so rank-0-only logging preserves all information.
         if (comm_.rank() == 0) {
-            OpmLog::info(msg);
+            opmlog_fn(msg);
         }
     }
 }
 
-void Logger::warning(const std::string &msg) const {
-    if (haveDeferredLogger()) {
-        // DeferredLogger: All ranks log - messages will be gathered later
-        this->deferred_logger_->warning(msg);
-    } else {
-        // OpmLog fallback: Only rank 0 logs (see comment in info() above)
-        if (comm_.rank() == 0) {
-            OpmLog::warning(msg);
-        }
-    }
-}
 
 // Seconds class alphabetically
 // ----------------------------

opm/simulators/flow/rescoup/ReservoirCoupling.hpp

@@ -48,12 +48,16 @@ public:
     void debug(const std::string &msg) const;
     DeferredLogger& deferredLogger() { return *deferred_logger_; }
     DeferredLogger& deferredLogger() const { return *deferred_logger_; }
+    void error(const std::string &msg) const;
     bool haveDeferredLogger() const { return deferred_logger_ != nullptr; }
     void info(const std::string &msg) const;
     void warning(const std::string &msg) const;
     void setDeferredLogger(DeferredLogger *deferred_logger) { deferred_logger_ = deferred_logger; }
 
 private:
+    template<typename DeferredFn, typename OpmLogFn>
+    void forward_(const std::string &msg, DeferredFn deferred_fn, OpmLogFn opmlog_fn) const;
+
     const Parallel::Communication& comm_;
     DeferredLogger *deferred_logger_ = nullptr;
 };
@@ -152,6 +156,7 @@ enum class MessageTag : int {
     SlaveProductionData,
     SlaveSimulationStartDate,
     SlaveStartOfReportStep,
+    SlaveStatus,
 };
 
 /// @brief Phase indices for reservoir coupling, we currently only support black-oil phases

opm/simulators/flow/rescoup/ReservoirCouplingSlave.cpp

@@ -368,6 +368,8 @@ void
 ReservoirCouplingSlave<Scalar>::
 sendAndReceiveInitialData() {
     // Communication order must match master's spawn() method.
+    // First, send status OK to let master know we initialized successfully.
+    this->sendStatusToMasterProcess_(/*ok=*/true);
     // We receive master group names before sending activation date so that
     // we can detect history matching mode (no master groups) and use start_date as activation.
     this->sendSimulationStartDateToMasterProcess_();
@@ -693,6 +695,24 @@ sendSimulationStartDateToMasterProcess_() const
    }
 }
 
+template <class Scalar>
+void
+ReservoirCouplingSlave<Scalar>::
+sendStatusToMasterProcess_(bool ok) {
+    if (this->comm_.rank() == 0) {
+        int status = ok ? 0 : 1;
+        MPI_Send(
+            &status,
+            /*count=*/1,
+            /*datatype=*/MPI_INT,
+            /*dest_rank=*/0,
+            /*tag=*/static_cast<int>(MessageTag::SlaveStatus),
+            this->slave_master_comm_
+        );
+        this->logger_.debug(fmt::format("Sent status {} to master", ok ? "OK" : "FAILED"));
+    }
+}
+
 // Explicit template instantiations
 template class ReservoirCouplingSlave<double>;
 #if FLOW_INSTANTIATE_FLOAT

opm/simulators/flow/rescoup/ReservoirCouplingSlave.hpp

@@ -198,6 +198,7 @@ private:
     void sendActivationDateToMasterProcess_();
     void sendActivationHandshakeToMasterProcess_() const;
     void sendSimulationStartDateToMasterProcess_() const;
+    void sendStatusToMasterProcess_(bool ok);
 
     const Parallel::Communication &comm_;
     const Schedule& schedule_;

opm/simulators/flow/rescoup/ReservoirCouplingSpawnSlaves.cpp

@@ -69,6 +69,7 @@ spawn()
     // Communication order is important - slaves need master group names
     // before they can determine their activation date (for history mode detection).
     this->spawnSlaveProcesses_();
+    this->receiveSlaveStatus_();
     this->receiveSimulationStartDateFromSlaves_();
     this->sendSlaveNamesToSlaves_();
     // These methods handle empty masterGroups gracefully (history mode)
@@ -264,6 +265,49 @@ receiveActivationDateFromSlaves_()
     this->logger_.debug("Broadcasted slave activation dates to all ranks");
 }
 
+template <class Scalar>
+void
+ReservoirCouplingSpawnSlaves<Scalar>::
+receiveSlaveStatus_()
+{
+    auto num_slaves = this->master_.numSlavesStarted();
+    if (this->comm_.rank() == 0) {
+        for (unsigned int i = 0; i < num_slaves; i++) {
+            int status = 0;
+            MPI_Recv(
+                &status,
+                /*count=*/1,
+                /*datatype=*/MPI_INT,
+                /*source_rank=*/0,
+                /*tag=*/static_cast<int>(MessageTag::SlaveStatus),
+                this->master_.getSlaveComm(i),
+                MPI_STATUS_IGNORE
+            );
+            if (status != 0) {
+                this->logger_.error(fmt::format(
+                    "Slave '{}' failed to initialize (parse error or other failure). "
+                    "The master cannot continue without all slaves.",
+                    this->master_.getSlaveName(i)
+                ));
+                // Disconnect from all slave communicators to avoid hanging
+                for (unsigned int j = 0; j < num_slaves; j++) {
+                    auto comm = this->master_.getSlaveComm(j);
+                    if (comm != MPI_COMM_NULL) {
+                        MPI_Comm_disconnect(&comm);
+                    }
+                }
+                RCOUP_LOG_THROW(std::runtime_error,
+                    "Reservoir coupling slave '" + this->master_.getSlaveName(i) +
+                    "' failed to initialize"
+                );
+            }
+            this->logger_.debug(fmt::format(
+                "Received OK status from slave '{}'", this->master_.getSlaveName(i)
+            ));
+        }
+    }
+}
+
 template <class Scalar>
 void
 ReservoirCouplingSpawnSlaves<Scalar>::

opm/simulators/flow/rescoup/ReservoirCouplingSpawnSlaves.hpp

@@ -59,6 +59,14 @@ private:
         std::string &log_filename) const;
     void receiveActivationDateFromSlaves_();
     void receiveSimulationStartDateFromSlaves_();
+    /// @brief Receive initialization status from each slave process.
+    /// @details After spawning, each slave sends a status code (0 = OK, non-zero = failed).
+    ///   If any slave reports failure, disconnects all slave communicators and throws
+    ///   std::runtime_error to prevent the master from hanging on subsequent MPI calls.
+    /// @note Currently only covers failures during readDeck() (parse errors). Exceptions
+    ///   between readDeck() and SimulatorFullyImplicitBlackoil::init() (where the slave
+    ///   sends its OK status) are not yet caught.
+    void receiveSlaveStatus_();
     void sendMasterGroupNamesToSlaves_();
     void sendSlaveNamesToSlaves_();
     void spawnSlaveProcesses_();

opm/simulators/utils/readDeck.cpp

@@ -767,6 +767,12 @@ void Opm::readDeck(Opm::Parallel::Communication    comm,
         const bool lowActionParsingStrictness = (actionParsingStrictness == "low");
         try {
             auto parseContext = setupParseContext(exitOnAllErrors);
+            if (slaveMode) {
+                // In slave mode, fatal parse errors must throw instead of calling
+                // std::exit() so the slave can notify the master before exiting.
+                // Without this, the master hangs on MPI_Recv waiting for the slave.
+                parseContext->update(ParseContext::PARSE_MISSING_INCLUDE, InputErrorAction::THROW_EXCEPTION);
+            }
             if (treatCriticalAsNonCritical) { // Continue with invalid names if parsing strictness is set to low
                 parseContext->update(ParseContext::SCHEDULE_INVALID_NAME, InputErrorAction::WARN);
             }

tests/rescoup/slave_parse_error/run_ctest.sh

@@ -17,16 +17,11 @@
 # along with OPM.  If not, see <http://www.gnu.org/licenses/>.
 
 # CTest wrapper for the reservoir coupling slave parse error test.
-# Exits 0 on expected (currently unfixed) behavior, 1 on unexpected behavior.
 #
-# The slave deck has a deliberate parse error. The master should never
+# The slave deck has a deliberate parse error. The slave notifies the master
-# complete successfully (exit 0). Expected outcomes depending on MPI:
+# of the failure via MPI, and the master exits with code 1 on all MPI
-#   - exit 1:   MPI runtime detected slave exit (OpenMPI 4.1.6)
+# implementations. Exit 0 (master completed despite slave parse error)
-#   - exit 9:   MPI runtime killed master when slave exited (custom MPICH v4.3.2)
+# is treated as FAIL.
-#   - exit 124: master hung on MPI_Recv until timeout (OpenMPI 5.0.8, system MPICH v4.2.1)
-#
-# Any non-zero exit code is treated as PASS. Only exit 0 (master completed
-# despite slave parse error) is treated as FAIL.
 #
 # Known issue — OpenMPI 5.x BTL TCP IPv6 bug (observed with Ubuntu package):
 #   On systems where a network interface has both IPv4 and IPv6 addresses
@@ -71,8 +66,7 @@ echo "MPI launcher: $MPI_LAUNCHER"
 echo "Working dir:  $(pwd)"
 echo ""
 
-# Run with a timeout as a safety net. Some MPI implementations detect the
+# Run with a timeout as a safety net in case of regression (master hanging).
-# slave exit immediately (exit 1 or 9), others hang until the timeout (exit 124).
 timeout 10 \
     "$MPI_LAUNCHER" -np 1 "$FLOW_BINARY" \
     RC_MASTER.DATA \
@@ -82,10 +76,13 @@ timeout 10 \
 
 EXIT_CODE=$?
 
-if [ $EXIT_CODE -eq 0 ]; then
+if [ $EXIT_CODE -eq 1 ]; then
+    echo "PASS: Master detected slave failure (exit code 1)"
+    exit 0
+elif [ $EXIT_CODE -eq 0 ]; then
     echo "FAIL: Master completed successfully — unexpected for slave parse error"
     exit 1
 else
-    echo "PASS: Master did not complete (exit code $EXIT_CODE)"
+    echo "FAIL: Unexpected exit code $EXIT_CODE (expected 1)"
-    exit 0
+    exit 1
 fi