Stop quit[] from ending the host process

app/forefire/ForeFire.cpp

@@ -85,12 +85,14 @@ int ForeFire::startShell(int argc, char* argv[]) {
         string listenCommand = "listenHTTP[]";
         executor.ExecuteCommand(listenCommand);
 
-        // Keep the main thread alive indefinitely
+        // Keep the main thread alive while the server runs. `quit[]` used to
+        // end the process from inside the library; now it asks, and this is
+        // the loop that answers, so a served quit[] still stops the server.
         cout << "(Press Ctrl+C to exit)" << endl;
-        while (true) {
-            sleep(3600);
+        while (!Command::quitRequested()) {
+            sleep(1);
         }
-        
+
         return 0;
 
     } else {
@@ -160,6 +162,10 @@ void ForeFire::FFShell(ifstream* inputStream) {
             if (line.empty())
                 continue;
             executor.ExecuteCommand(line);
+            // `quit[]` no longer ends the process from inside the library, so
+            // the shell is what leaves on request.
+            if (Command::quitRequested())
+                break;
         }
     }
 }

src/Command.cpp

@@ -29,6 +29,7 @@ namespace libforefire
     double Command::endTime = 0;
 
     bool Command::firstCommand = true;
+    bool Command::quitAsked = false;
     size_t Command::refTabs = 0;
 
     FFPoint *Command::lastReadLoc = 0;
@@ -1116,13 +1117,14 @@ namespace libforefire
                 }
                 catch (...)
                 {
-                    if (getDomain()->commandOutputs)
-                    {
-                        cout << getDomain()->getDomainID() << ": "
-                             << "**** ERROR IN SAFE TOPOLOGY MODE, QUITING ****" << endl;
-                    }
-                    // TODO supersafe mode ?
-                    quit(arg, numTabs);
+                    // Reports an error rather than ending the process. This
+                    // used to call quit(), so an internal failure here took
+                    // the host down with exit status 0 — a coupled run or a
+                    // batch job recorded success while having stopped early.
+                    // The caller now sees the failure and chooses.
+                    cerr << getDomain()->getDomainID() << ": "
+                         << "**** ERROR IN SAFE TOPOLOGY MODE ****" << endl;
+                    return error;
                 }
             }
         }
@@ -2423,6 +2425,11 @@ namespace libforefire
 
         while (std::getline(*inputStream, line))
         {
+            // A `quit[]` earlier in the script stops the file here rather than
+            // ending the process, so the caller still gets to return normally.
+            if (quitAsked)
+                break;
+
             if (!inTripleQuote)
             {
                 /* Skip comments and empty lines when *not* inside a multiline literal   */
@@ -2959,15 +2966,40 @@ namespace libforefire
 
     int Command::quit(const string &arg, size_t &numTabs)
     {
-
+        // No exit() here. `quit` is in the command table, so it is reachable
+        // from the Python binding and from the HTTP server, and a library
+        // that ends its host's process skips every destructor, `finally` and
+        // atexit handler on the way out — and used to do it with status 0, so
+        // a batch run reported success having done nothing. The session is
+        // released and the request is recorded; whoever is driving the shell
+        // decides what to do about it.
         delete currentSession.fd;
+        currentSession.fd = 0;
         delete currentSession.outStrRep;
+        currentSession.outStrRep = 0;
         delete currentSession.sim;
-        delete currentSession.params;
-        exit(0);
+        currentSession.sim = 0;
+
+        // currentSession.params is deliberately not deleted: it is the
+        // SimulationParameters singleton, owned by GetInstance() and shared
+        // with every other holder. Deleting it left GetInstance() handing out
+        // a dangling pointer, which only went unnoticed because exit()
+        // followed on the next line.
+
+        quitAsked = true;
         return normal;
     }
 
+    bool Command::quitRequested()
+    {
+        return quitAsked;
+    }
+
+    void Command::clearQuitRequest()
+    {
+        quitAsked = false;
+    }
+
     void Command::setOstringstream(ostringstream *oss)
     {
         currentSession.outStream = oss;

src/Command.h

@@ -121,6 +121,8 @@ class Command {
 	};
 
 	static bool firstCommand;
+	/*! \brief set by quit(), read by quitRequested() */
+	static bool quitAsked;
 	static size_t refTabs;
 
 	static FFPoint* lastReadLoc;
@@ -176,7 +178,14 @@ class Command {
 	static int systemExec(const string&, size_t&);
 	/*! \brief command to run a system trough pipe */
 	static int clear(const string&, size_t&);
-	/*! \brief command to quit the ForeFire shell */
+	/*! \brief releases the session and asks the host to stop
+	 *
+	 * Does NOT terminate the process. This is library code, and the host
+	 * process is not ours to end: a `quit[]` arriving over the Python binding
+	 * or the HTTP server used to take the interpreter down with it, skipping
+	 * every destructor and `finally`, and exiting 0 so a batch job reported
+	 * success. Callers driving a shell should check quitRequested() and stop.
+	 */
 	static int quit(const string&, size_t&);
     /*! \brief command to quit the ForeFire shell */
     static int listenHTTP(const string&, size_t &) ;
@@ -232,6 +241,16 @@ class Command {
 
 public:
 
+	/*! \brief whether a `quit[]` has asked the host to stop
+	 *
+	 * Set by quit() and cleared by clearQuitRequest(). A shell reading
+	 * commands should stop its loop when this is true; a library embedder is
+	 * free to ignore it and carry on, which is the difference between asking
+	 * and terminating. */
+	static bool quitRequested();
+	/*! \brief forgets a pending quit request */
+	static void clearQuitRequest();
+
 	// Reference time
 	static double refTime;
 

tests/unit/CMakeLists.txt

@@ -12,7 +12,8 @@ add_executable(forefire_unit_tests
     test_flux_models.cpp
     test_http_server.cpp
     test_model_registry.cpp
-    test_propagation_models.cpp)
+    test_propagation_models.cpp
+    test_quit_command.cpp)
 
 target_link_libraries(forefire_unit_tests PRIVATE forefireL)
 target_include_directories(forefire_unit_tests PRIVATE
@@ -25,8 +26,15 @@ endif()
 
 # doctest groups its cases into suites; registering one CTest entry per suite
 # keeps `ctest` output useful without needing doctest's CMake integration.
-foreach(_suite "model registry" "propagation models" "flux models" "http server")
+foreach(_suite "model registry" "propagation models" "flux models" "http server"
+                "quit command")
     string(REPLACE " " "_" _suite_id "${_suite}")
     add_test(NAME "unit.${_suite_id}"
              COMMAND forefire_unit_tests --test-suite=${_suite} --no-skipped-summary)
+    # Requiring doctest's closing line, not just a zero exit status. A test
+    # that ends the process mid-run — which is exactly what quit[] used to do,
+    # with exit(0) — otherwise reports as a pass, because ctest sees 0 and the
+    # summary that never printed is not something it looks for.
+    set_tests_properties("unit.${_suite_id}" PROPERTIES
+                         PASS_REGULAR_EXPRESSION "\\[doctest\\] Status: SUCCESS!")
 endforeach()

tests/unit/test_quit_command.cpp

@@ -0,0 +1,75 @@
+/**
+ * @file test_quit_command.cpp
+ * @brief `quit[]` must not end the process it is running inside.
+ * @copyright Copyright (C) 2025 ForeFire, Fire Team, SPE, CNRS/Universita di Corsica.
+ * @license This program is free software; See LICENSE file for details. (See LICENSE file).
+ *
+ * `quit` sits in the script command table, so it is reachable from the Python
+ * binding and from the HTTP server, and it used to call exit(0). A host got no
+ * traceback, no exception, no `finally`, no destructors, and a status of 0 —
+ * a batch job recorded success having stopped early. See #160.
+ *
+ * The test for that is the whole file running to completion: if `quit[]` still
+ * called exit(0), the cases below this one would not report, and ctest would
+ * see a suite that passed nothing rather than a suite that failed. The
+ * `--test-suite` entry in CMakeLists is what turns that into a visible result.
+ */
+
+#include "doctest/doctest.h"
+
+#include "Command.h"
+
+#include <string>
+
+using libforefire::Command;
+
+TEST_SUITE("quit command") {
+
+TEST_CASE("quit[] returns instead of ending the process") {
+    Command executor;
+    Command::clearQuitRequest();
+
+    std::string command = "quit[]";
+    executor.ExecuteCommand(command);
+
+    // Reaching this line at all is the assertion that matters.
+    CHECK(Command::quitRequested());
+}
+
+TEST_CASE("the quit request can be cleared and the session reused") {
+    // A host that decides to ignore the request has to be able to carry on,
+    // which is the difference between asking and terminating.
+    Command executor;
+    Command::clearQuitRequest();
+    REQUIRE_FALSE(Command::quitRequested());
+
+    std::string quitCommand = "quit[]";
+    executor.ExecuteCommand(quitCommand);
+    REQUIRE(Command::quitRequested());
+
+    Command::clearQuitRequest();
+    CHECK_FALSE(Command::quitRequested());
+
+    // The interpreter still works after a quit: setParameter and getParameter
+    // round-trip, which they could not do if quit had freed the parameters
+    // singleton as it used to.
+    std::string setCommand = "setParameter[ff_test.after.quit=42]";
+    executor.ExecuteCommand(setCommand);
+    CHECK(libforefire::SimulationParameters::GetInstance()
+              ->getParameter("ff_test.after.quit") == "42");
+}
+
+TEST_CASE("a second quit is harmless") {
+    // quit() deletes the session objects and nulls them; running it twice must
+    // not double free what the first call released.
+    Command executor;
+    Command::clearQuitRequest();
+
+    std::string command = "quit[]";
+    executor.ExecuteCommand(command);
+    executor.ExecuteCommand(command);
+
+    CHECK(Command::quitRequested());
+}
+
+} /* TEST_SUITE */