wsd: simplify shutdown and termination flagging

With the use of a single flag for both, the
logic is now less ambiguous, as we cannot have
termination flagged without also implying
shutting down.
The assertions are no longer needed.

Now that setting the termination flag
explicitly implies having the shut down flag
as well, the checks are simpler. We only
need to check that the shutdown is not set
to continue running as normal, since having
the termination flag must perfoce mean shut
down is also set, there is no need to check
both.

Change-Id: I99e22f5668385182b0594040a8e3354b55e74642
Signed-off-by: Ashod Nakashian <ashod.nakashian@collabora.co.uk>

common/SigUtil.cpp

@@ -41,8 +41,18 @@
 namespace
 {
 #ifndef IOS
-static std::atomic<bool> TerminationFlag(false);
-static std::atomic<bool> ShutdownRequestFlag(false);
+
+/// The valid states of the process.
+enum class RunState : char
+{
+    Run = 0, //< Normal up-and-running state.
+    ShutDown, //< Request to shut down gracefully.
+    Terminate //< Immediate termination.
+};
+
+/// Single flag to control the current run state.
+static std::atomic<RunState> RunStateFlag(RunState::Run);
+
 #if !MOBILEAPP
 static std::atomic<bool> DumpGlobalState(false);
 static std::atomic<bool> ForwardSigUsr2Flag(false); //< Flags to forward SIG_USR2 to children.
@@ -61,41 +71,30 @@ static char FatalGdbString[256] = { '\0' };
 namespace SigUtil
 {
 #ifndef IOS
-    bool getShutdownRequestFlag()
-    {
-        // ShutdownRequestFlag must be set if TerminationFlag is set.
-        assert(!TerminationFlag || ShutdownRequestFlag);
-        return ShutdownRequestFlag;
-    }
+bool getShutdownRequestFlag() { return RunStateFlag >= RunState::ShutDown; }
 
-    bool getTerminationFlag()
-    {
-        // ShutdownRequestFlag must be set if TerminationFlag is set.
-        assert(!TerminationFlag || ShutdownRequestFlag);
-        return TerminationFlag;
-    }
+bool getTerminationFlag() { return RunStateFlag >= RunState::Terminate; }
+
+void setTerminationFlag()
+{
+    // Set the forced-termination flag.
+    RunStateFlag = RunState::Terminate;
 
-    void setTerminationFlag()
-    {
-#if !MOBILEAPP
-        // Request shutting down first. Otherwise, we can race with
-        // getTerminationFlag, which asserts ShutdownRequestFlag.
-        ShutdownRequestFlag = true;
-#endif
-        // Set the forced-termination flag.
-        TerminationFlag = true;
 #if !MOBILEAPP
         // And wake-up the thread.
         SocketPoll::wakeupWorld();
 #endif
-    }
+}
+
+void requestShutdown()
+{
+    RunState oldState = RunState::Run;
+    if (RunStateFlag.compare_exchange_strong(oldState, RunState::ShutDown))
+        SocketPoll::wakeupWorld();
+}
 
 #if MOBILEAPP
-    void resetTerminationFlags()
-    {
-        TerminationFlag = false;
-        ShutdownRequestFlag = false;
-    }
+    void resetTerminationFlags() { RunStateFlag = RunState::Run; }
 #endif
 #endif // !IOS
 
@@ -303,21 +302,27 @@ namespace SigUtil
         const auto onrre = errno; // Save.
 
         bool hardExit = false;
-        const char *domain;
-        if (!ShutdownRequestFlag && (signal == SIGINT || signal == SIGTERM))
+        const char* domain;
+        RunState oldState = RunState::Run;
+        if ((signal == SIGINT || signal == SIGTERM) &&
+            RunStateFlag.compare_exchange_strong(oldState, RunState::ShutDown))
         {
             domain = " Shutdown signal received: ";
-            ShutdownRequestFlag = true;
-        }
-        else if (!TerminationFlag)
-        {
-            domain = " Forced-Termination signal received: ";
-            TerminationFlag = true;
         }
         else
         {
-            domain = " ok, ok - hard-termination signal received: ";
-            hardExit = true;
+            assert(RunStateFlag > RunState::Run && "Must have had Terminate flag");
+            oldState = RunState::ShutDown;
+            if (RunStateFlag.compare_exchange_strong(oldState, RunState::Terminate))
+            {
+                domain = " Forced-Termination signal received: ";
+            }
+            else
+            {
+                assert(RunStateFlag == RunState::Terminate && "Must have had Terminate flag");
+                domain = " ok, ok - hard-termination signal received: ";
+                hardExit = true;
+            }
         }
 
         signalLogOpen();
@@ -343,12 +348,6 @@ namespace SigUtil
         errno = onrre; // Restore.
     }
 
-    void requestShutdown()
-    {
-        ShutdownRequestFlag = true;
-        SocketPoll::wakeupWorld();
-    }
-
     static
     void handleFatalSignal(const int signal, siginfo_t *info, void * /* uctxt */)
     {

kit/ForKit.cpp

@@ -757,7 +757,7 @@ int main(int argc, char** argv)
     const int parentPid = getppid();
     LOG_INF("ForKit process is ready. Parent: " << parentPid);
 
-    while (!SigUtil::getShutdownRequestFlag() && !SigUtil::getTerminationFlag())
+    while (!SigUtil::getShutdownRequestFlag())
     {
         UnitKit::get().invokeForKitTest();
 

wsd/Admin.cpp

@@ -489,7 +489,7 @@ void Admin::pollingThread()
     std::chrono::steady_clock::time_point lastNet = lastCPU;
     std::chrono::steady_clock::time_point lastCleanup = lastCPU;
 
-    while (!isStop() && !SigUtil::getTerminationFlag() && !SigUtil::getShutdownRequestFlag())
+    while (!isStop() && !SigUtil::getShutdownRequestFlag())
     {
         const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now();
 

wsd/COOLWSD.cpp

@@ -1116,7 +1116,7 @@ public:
 
     void pollingThread()
     {
-        while (!isStop() && !SigUtil::getTerminationFlag() && !SigUtil::getShutdownRequestFlag())
+        while (!isStop() && !SigUtil::getShutdownRequestFlag())
         {
             Poco::URI remoteServerURI(_conf.getString(_configKey));
 
@@ -3015,7 +3015,7 @@ bool COOLWSD::checkAndRestoreForKit()
     if (ForKitProcId == -1)
     {
         // Fire the ForKit process for the first time.
-        if (!SigUtil::getShutdownRequestFlag() && !SigUtil::getTerminationFlag() && !createForKit())
+        if (!SigUtil::getShutdownRequestFlag() && !createForKit())
         {
             // Should never fail.
             LOG_FTL("Setting ShutdownRequestFlag: Failed to spawn coolforkit.");
@@ -3044,7 +3044,7 @@ bool COOLWSD::checkAndRestoreForKit()
                 }
 
                 // Spawn a new forkit and try to dust it off and resume.
-                if (!SigUtil::getShutdownRequestFlag() && !SigUtil::getTerminationFlag() && !createForKit())
+                if (!SigUtil::getShutdownRequestFlag() && !createForKit())
                 {
                     LOG_FTL("Setting ShutdownRequestFlag: Failed to spawn forkit instance.");
                     SigUtil::requestShutdown();
@@ -3078,7 +3078,7 @@ bool COOLWSD::checkAndRestoreForKit()
         {
             // No child processes.
             // Spawn a new forkit and try to dust it off and resume.
-            if (!SigUtil::getShutdownRequestFlag() && !SigUtil::getTerminationFlag() && !createForKit())
+            if (!SigUtil::getShutdownRequestFlag()  && !createForKit())
             {
                 LOG_FTL("Setting ShutdownRequestFlag: Failed to spawn forkit instance.");
                 SigUtil::requestShutdown();
@@ -5805,7 +5805,7 @@ int COOLWSD::innerMain()
     auto stampFetch = startStamp - (fetchUpdateCheck - std::chrono::milliseconds(60000));
 #endif
 
-    while (!SigUtil::getTerminationFlag() && !SigUtil::getShutdownRequestFlag())
+    while (!SigUtil::getShutdownRequestFlag())
     {
         // This timeout affects the recovery time of prespawned children.
         std::chrono::microseconds waitMicroS = SocketPoll::DefaultPollTimeoutMicroS * 4;

wsd/DocumentBroker.cpp

@@ -214,8 +214,7 @@ void DocumentBroker::pollThread()
 
         // Nominal time between retries, lest we busy-loop. getNewChild could also wait, so don't double that here.
         std::this_thread::sleep_for(std::chrono::milliseconds(CHILD_REBALANCE_INTERVAL_MS / 10));
-    } while (!_stop && _poll->continuePolling() && !SigUtil::getTerminationFlag() &&
-             !SigUtil::getShutdownRequestFlag());
+    } while (!_stop && _poll->continuePolling() && !SigUtil::getShutdownRequestFlag());
 
     if (!_childProcess)
     {

wsd/DocumentBroker.hpp

@@ -407,8 +407,7 @@ public:
     bool isUnloading() const
     {
         return _docState.isMarkedToDestroy() || _stop || _docState.isUnloadRequested() ||
-               _docState.isCloseRequested() || SigUtil::getShutdownRequestFlag() ||
-               SigUtil::getTerminationFlag();
+               _docState.isCloseRequested() || SigUtil::getShutdownRequestFlag();
     }
 
     bool isMarkedToDestroy() const { return _docState.isMarkedToDestroy() || _stop; }