Merge pull request #15730 from pkerling/fix-posix-signal

Handle signals by setting atomic flag instead of  pop-up thread

xbmc/Application.cpp

@@ -171,6 +171,7 @@
 #include "XHandle.h"
 #include "XTimeUtils.h"
 #include "platform/posix/filesystem/PosixDirectory.h"
+#include "platform/posix/PlatformPosix.h"
 #endif
 
 #if defined(TARGET_ANDROID)
@@ -4106,6 +4107,14 @@ void CApplication::ProcessSlow()
     CheckShutdown();
   }
 
+#if defined(TARGET_POSIX)
+  if (CPlatformPosix::TestShutdownFlag())
+  {
+    CLog::Log(LOGNOTICE, "Shutting down due to POSIX signal");
+    CApplicationMessenger::GetInstance().PostMsg(TMSG_SHUTDOWN);
+  }
+#endif
+
   // check if we should restart the player
   CheckDelayedPlayerRestart();
 

xbmc/platform/darwin/PlatformDarwin.cpp

@@ -22,5 +22,6 @@ CPlatformDarwin::~CPlatformDarwin()
 
 void CPlatformDarwin::Init()
 {
-    setenv("SSL_CERT_FILE", CSpecialProtocol::TranslatePath("special://xbmc/system/certs/cacert.pem").c_str(), 0);
+  CPlatformPosix::Init();
+  setenv("SSL_CERT_FILE", CSpecialProtocol::TranslatePath("special://xbmc/system/certs/cacert.pem").c_str(), 0);
 }

xbmc/platform/darwin/PlatformDarwin.h

@@ -8,9 +8,9 @@
 
 #pragma once
 
-#include "platform/Platform.h"
+#include "platform/posix/PlatformPosix.h"
 
-class CPlatformDarwin : public CPlatform
+class CPlatformDarwin : public CPlatformPosix
 {
   public:
     /**\brief C'tor */

xbmc/platform/overrides/android/PlatformAndroid.cpp

@@ -27,5 +27,6 @@ CPlatformAndroid::~CPlatformAndroid()
 
 void CPlatformAndroid::Init()
 {
-    setenv("SSL_CERT_FILE", CSpecialProtocol::TranslatePath("special://xbmc/system/certs/cacert.pem").c_str(), 1);
+  CPlatformPosix::Init();
+  setenv("SSL_CERT_FILE", CSpecialProtocol::TranslatePath("special://xbmc/system/certs/cacert.pem").c_str(), 1);
 }

xbmc/platform/overrides/android/PlatformAndroid.h

@@ -8,9 +8,9 @@
 
 #pragma once
 
-#include "platform/Platform.h"
+#include "platform/posix/PlatformPosix.h"
 
-class CPlatformAndroid : public CPlatform
+class CPlatformAndroid : public CPlatformPosix
 {
   public:
     /**\brief C'tor */

xbmc/platform/overrides/freebsd/PlatformPosix.cpp

@@ -0,0 +1,14 @@
+/*
+ *  Copyright (C) 2019 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "platform/posix/PlatformPosix.h"
+
+CPlatform* CPlatform::CreateInstance()
+{
+  return new CPlatformPosix();
+}

xbmc/platform/overrides/linux/PlatformPosix.cpp

@@ -0,0 +1,14 @@
+/*
+ *  Copyright (C) 2019 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "platform/posix/PlatformPosix.h"
+
+CPlatform* CPlatform::CreateInstance()
+{
+  return new CPlatformPosix();
+}

xbmc/platform/posix/CMakeLists.txt

@@ -1,4 +1,7 @@
 set(SOURCES Filesystem.cpp
-            MessagePrinter.cpp)
+            MessagePrinter.cpp
+            PlatformPosix.cpp)
+
+set(HEADERS PlatformPosix.h)
 
 core_add_library(platform_posix)

xbmc/platform/posix/PlatformPosix.cpp

@@ -0,0 +1,28 @@
+/*
+ *  Copyright (C) 2019 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#include "PlatformPosix.h"
+
+std::atomic_flag CPlatformPosix::ms_signalFlag;
+
+void CPlatformPosix::Init()
+{
+  // Initialize to "set" state
+  ms_signalFlag.test_and_set();
+}
+
+bool CPlatformPosix::TestShutdownFlag()
+{
+  // Keep set, return true when it was cleared before
+  return !ms_signalFlag.test_and_set();
+}
+
+void CPlatformPosix::RequestShutdown()
+{
+  ms_signalFlag.clear();
+}

xbmc/platform/posix/PlatformPosix.h

@@ -0,0 +1,25 @@
+/*
+ *  Copyright (C) 2019 Team Kodi
+ *  This file is part of Kodi - https://kodi.tv
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ *  See LICENSES/README.md for more information.
+ */
+
+#pragma once
+
+#include <atomic>
+
+#include "platform/Platform.h"
+
+class CPlatformPosix : public CPlatform
+{
+public:
+  void Init() override;
+
+  static bool TestShutdownFlag();
+  static void RequestShutdown();
+
+private:
+  static std::atomic_flag ms_signalFlag;
+};

xbmc/platform/posix/main.cpp

@@ -6,8 +6,8 @@
  *  See LICENSES/README.md for more information.
  */
 
-#include <sys/resource.h>
 #include <signal.h>
+#include <sys/resource.h>
 
 #include <cstring>
 
@@ -25,6 +25,7 @@
 #include "PlayListPlayer.h"
 #include "platform/MessagePrinter.h"
 #include "platform/xbmc.h"
+#include "PlatformPosix.h"
 #include "utils/log.h"
 
 #ifdef HAS_LIRC
@@ -36,31 +37,14 @@
 namespace
 {
 
-class CPOSIXSignalHandleThread : public CThread
-{
-public:
-  CPOSIXSignalHandleThread()
-  : CThread("POSIX signal handler")
-  {}
-protected:
-  void Process() override
-  {
-    CMessagePrinter::DisplayMessage("Exiting application");
-    KODI::MESSAGING::CApplicationMessenger::GetInstance().PostMsg(TMSG_QUIT);
-  }
-};
-
 extern "C"
 {
 
 void XBMC_POSIX_HandleSignal(int sig)
 {
-  // Spawn handling thread: the current thread that this signal was catched on
-  // might have been interrupted in a call to PostMsg() while holding a lock
-  // there, which would lead to a deadlock if PostMsg() was called directly here
-  // as PostMsg() is not supposed to be reentrant
-  auto thread = new CPOSIXSignalHandleThread;
-  thread->Create(true);
+  // Setting an atomic flag is one of the only useful things that is permitted by POSIX
+  // in signal handlers
+  CPlatformPosix::RequestShutdown();
 }
 
 }