[Libreoffice-commits] online.git: common/SigUtil.cpp common/SigUtil.hpp common/Unit.cpp kit/ForKit.cpp kit/Kit.cpp wsd/Admin.cpp wsd/LOOLWSD.cpp

Ashod Nakashian (via logerrit) logerrit at kemper.freedesktop.org
Wed Nov 6 02:44:06 UTC 2019


 common/SigUtil.cpp |   42 ++++++++++++++++++++++++++----------------
 common/SigUtil.hpp |   38 ++++++++++++++++++--------------------
 common/Unit.cpp    |    2 +-
 kit/ForKit.cpp     |    2 +-
 kit/Kit.cpp        |    6 +++---
 wsd/Admin.cpp      |    3 +--
 wsd/LOOLWSD.cpp    |    6 +++---
 7 files changed, 53 insertions(+), 46 deletions(-)

New commits:
commit 7a976488f08bbeeb5620639cd47ad63afb030aed
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sun Nov 3 20:52:19 2019 -0500
Commit:     Ashod Nakashian <ashnakash at gmail.com>
CommitDate: Wed Nov 6 03:43:45 2019 +0100

    wsd: cleanup the global flag accessors
    
    The following flags are affected:
    ShutdownRequestFlag
    TerminationFlag
    DumpGlobalState
    
    Since it's common to grep for all places
    that set or reset these global flags, it
    makes more sense to have explicit functions
    for each operation. Now we have set and reset
    accessors where appropriate and get is reserved
    for read-only access.
    
    This changes the getters to only return
    the boolean value of these flags rather than
    a reference to the atomic object, now that
    they are read-only.
    
    Also, a few Mobile-specific cases were folded
    either with other Mobile-specific sections, or
    they were now identical to the non-Mobile case
    and therefore deduplicated, making the code
    cleaner and more readable.
    
    Change-Id: Icc852aa43e86695d4e7d5962040a9b5086d9d08c
    Reviewed-on: https://gerrit.libreoffice.org/81978
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/common/SigUtil.cpp b/common/SigUtil.cpp
index 933ad7f3e..212d1382f 100644
--- a/common/SigUtil.cpp
+++ b/common/SigUtil.cpp
@@ -40,39 +40,49 @@
 
 static std::atomic<bool> TerminationFlag(false);
 static std::atomic<bool> DumpGlobalState(false);
+#if MOBILEAPP
+std::atomic<bool> MobileTerminationFlag(false);
+#else
+// Mobile defines its own, which is constexpr.
+static std::atomic<bool> ShutdownRequestFlag(false);
+#endif
+
 namespace SigUtil
 {
-    std::atomic<bool>& getTerminationFlag()
+    bool getShutdownRequestFlag()
+    {
+        return ShutdownRequestFlag;
+    }
+
+    bool getTerminationFlag()
     {
         return TerminationFlag;
     }
-    std::atomic<bool>& getDumpGlobalState()
+
+    void setTerminationFlag()
     {
-        return DumpGlobalState;
+        TerminationFlag = true;
     }
-}
 
 #if MOBILEAPP
-std::atomic<bool> MobileTerminationFlag(false);
-namespace SigUtil
-{
-    bool getShutdownRequestFlag()
+    void resetTerminationFlag()
     {
-        return ShutdownRequestFlag;
+        TerminationFlag = false;
     }
-}
 #endif
 
-#if !MOBILEAPP
-static std::atomic<bool> ShutdownRequestFlag(false);
-namespace SigUtil
-{
-    std::atomic<bool>& getShutdownRequestFlag()
+    bool getDumpGlobalState()
     {
-        return ShutdownRequestFlag;
+        return DumpGlobalState;
+    }
+
+    void resetDumpGlobalState()
+    {
+        DumpGlobalState = false;
     }
 }
 
+#if !MOBILEAPP
 std::mutex SigHandlerTrap;
 
 namespace SigUtil
diff --git a/common/SigUtil.hpp b/common/SigUtil.hpp
index 723dd2e5e..caf8d531d 100644
--- a/common/SigUtil.hpp
+++ b/common/SigUtil.hpp
@@ -13,36 +13,34 @@
 #include <atomic>
 #include <mutex>
 
-#if !MOBILEAPP
-namespace SigUtil
-{
-    /// Flag to commence clean shutdown
-    std::atomic<bool>& getShutdownRequestFlag();
-}
-#else
+#if MOBILEAPP
 static constexpr bool ShutdownRequestFlag(false);
-namespace SigUtil
-{
-    /// Flag to commence clean shutdown
-    bool getShutdownRequestFlag();
-}
+extern std::atomic<bool> MobileTerminationFlag;
 #endif
 
 namespace SigUtil
 {
-    /// Flag to stop pump loops.
-    std::atomic<bool>& getTerminationFlag();
-
-    /// Flag to dump internal state
-    std::atomic<bool>& getDumpGlobalState();
-}
+    /// Get the flag used to commence clean shutdown.
+    /// requestShutdown() is used to set the flag.
+    bool getShutdownRequestFlag();
 
+    /// Get the flag to stop pump loops forcefully.
+    bool getTerminationFlag();
+    /// Set the flag to stop pump loops forcefully.
+    void setTerminationFlag();
 #if MOBILEAPP
-extern std::atomic<bool> MobileTerminationFlag;
+    /// Reset the flag to stop pump loops forcefully.
+    /// Only necessary in Mobile.
+    void resetTerminationFlag();
 #endif
 
-#if !MOBILEAPP
+    /// Get the flag to dump internal state.
+    bool getDumpGlobalState();
+    /// Reset the flag to dump internal state.
+    void resetDumpGlobalState();
+}
 
+#if !MOBILEAPP
 namespace SigUtil
 {
     /// Mutex to trap signal handler, if any,
diff --git a/common/Unit.cpp b/common/Unit.cpp
index 1dbe09645..151a4505d 100644
--- a/common/Unit.cpp
+++ b/common/Unit.cpp
@@ -189,7 +189,7 @@ void UnitBase::exitTest(TestResult result)
     _retValue = result == TestResult::Ok ?
         Poco::Util::Application::EXIT_OK :
         Poco::Util::Application::EXIT_SOFTWARE;
-    SigUtil::getTerminationFlag() = true;
+    SigUtil::setTerminationFlag();
     SocketPoll::wakeupWorld();
 }
 
diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index 2eea47725..c6e2ebd4a 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -77,7 +77,7 @@ public:
     bool pollAndDispatch()
     {
         std::string message;
-        const int ready = readLine(message, [](){ return SigUtil::getTerminationFlag().load(); });
+        const int ready = readLine(message, [](){ return SigUtil::getTerminationFlag(); });
         if (ready <= 0)
         {
             // Termination is done via SIGTERM, which breaks the wait.
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 5dede6dd9..6ae7e0eec 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -2146,7 +2146,7 @@ protected:
         else if (tokens[0] == "exit")
         {
             LOG_INF("Setting TerminationFlag due to 'exit' command from parent.");
-            SigUtil::getTerminationFlag() = true;
+            SigUtil::setTerminationFlag();
             document.reset();
         }
         else if (tokens[0] == "tile" || tokens[0] == "tilecombine" || tokens[0] == "canceltiles" ||
@@ -2182,7 +2182,7 @@ protected:
     {
 #if !MOBILEAPP
         LOG_WRN("Kit connection lost without exit arriving from wsd. Setting TerminationFlag");
-        SigUtil::getTerminationFlag() = true;
+        SigUtil::setTerminationFlag();
 #endif
     }
 };
@@ -2260,7 +2260,7 @@ public:
         if (document && document->purgeSessions() == 0)
         {
             LOG_INF("Last session discarded. Setting TerminationFlag");
-            SigUtil::getTerminationFlag() = true;
+            SigUtil::setTerminationFlag();
             return -1;
         }
 #endif
diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index 319503b69..8a1d858b8 100644
--- a/wsd/Admin.cpp
+++ b/wsd/Admin.cpp
@@ -191,8 +191,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
     else if (tokens[0] == "shutdown")
     {
         LOG_INF("Shutdown requested by admin.");
-        SigUtil::getShutdownRequestFlag() = true;
-        SocketPoll::wakeupWorld();
+        SigUtil::requestShutdown();
         return;
     }
     else if (tokens[0] == "set" && tokens.count() > 1)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 9eb332b86..b8b826568 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1578,7 +1578,7 @@ void PrisonerPoll::wakeupHook()
                 replayThread->join();
 
                 LOG_INF("Setting TerminationFlag");
-                SigUtil::getTerminationFlag() = true;
+                SigUtil::setTerminationFlag();
             }
 #endif
         }
@@ -3208,7 +3208,7 @@ private:
             if (SigUtil::getDumpGlobalState())
             {
                 dump_state();
-                SigUtil::getDumpGlobalState() = false;
+                SigUtil::resetDumpGlobalState();
             }
         }
     };
@@ -3617,7 +3617,7 @@ void LOOLWSD::cleanup()
 int LOOLWSD::main(const std::vector<std::string>& /*args*/)
 {
 #if MOBILEAPP
-    SigUtil::getTerminationFlag() = false;
+    SigUtil::resetTerminationFlag();
 #endif
 
     int returnValue;


More information about the Libreoffice-commits mailing list