AndroidTestRunner: fix failure to acquire test runner semaphore

Currently, if the test runner is interrupted by SIGINT and SIGTERM,
the semaphore is never released and thus consecutive test runner
executions will wait forever. To fix that, a signal handler for the
former signals is added to release the QSystemSemaphore. Also,
remove RunnerLocker and simply use QSystemSemaphore directly,
and explicitly release the semaphore at the end of the test runner
execution.

Fixes: QTBUG-115298
Pick-to: 6.6 6.5
Change-Id: I4f6cc0e4d837460de9a66248e09d3cbaf24ce959
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
Reviewed-by: Zoltan Gera <zoltan.gera@qt.io>

1 files changed, 41 insertions, 15 deletions

diff --git a/src/tools/androidtestrunner/main.cpp b/src/tools/androidtestrunner/main.cpp
index fb4e641390..d24c694abc 100644
--- a/src/tools/androidtestrunner/main.cpp
+++ b/src/tools/androidtestrunner/main.cpp
@@ -11,6 +11,8 @@
 
 #include <algorithm>
 #include <functional>
+#include <atomic>
+#include <csignal>
 #include <QtCore/QDeadlineTimer>
 #include <QtCore/QThread>
 
@@ -496,20 +498,6 @@ static bool pullFiles()
     return ret;
 }
 
-struct RunnerLocker
-{
-    RunnerLocker()
-    {
-        runner.acquire();
-    }
-    ~RunnerLocker()
-    {
-        runner.release();
-    }
-    QSystemSemaphore runner{ QSystemSemaphore::platformSafeKey(u"androidtestrunner"_s),
-                1, QSystemSemaphore::Open };
-};
-
 void printLogcat(const QString &formattedTime)
 {
     QString logcatCmd = "%1 logcat "_L1.arg(g_options.adbCommand);
@@ -601,8 +589,41 @@ static QString getCurrentTimeString()
     return QString::fromUtf8(output.simplified());
 }
 
+struct TestRunnerSystemSemaphore
+{
+    TestRunnerSystemSemaphore() { }
+    ~TestRunnerSystemSemaphore() { release(); }
+
+    void acquire() { isAcquired.store(semaphore.acquire()); }
+
+    void release()
+    {
+        bool expected = true;
+        // NOTE: There's still could be tiny time gap between the compare_exchange_strong() call
+        // and release() call where the thread could be interrupted, if that's ever an issue,
+        // this code could be checked and improved further.
+        if (isAcquired.compare_exchange_strong(expected, false))
+            isAcquired.store(!semaphore.release());
+    }
+
+    std::atomic<bool> isAcquired { false };
+    QSystemSemaphore semaphore { QSystemSemaphore::platformSafeKey(u"androidtestrunner"_s),
+                                   1, QSystemSemaphore::Open };
+};
+
+TestRunnerSystemSemaphore testRunnerLock;
+
+void sigHandler(int signal)
+{
+    std::signal(signal, SIG_DFL);
+    testRunnerLock.release();
+}
+
 int main(int argc, char *argv[])
 {
+    std::signal(SIGINT,  sigHandler);
+    std::signal(SIGTERM, sigHandler);
+
     QCoreApplication a(argc, argv);
     if (!parseOptions()) {
         printHelp();
@@ -640,7 +661,9 @@ int main(int argc, char *argv[])
 
     obtainSDKVersion();
 
-    RunnerLocker lock; // do not install or run packages while another test is running
+    // do not install or run packages while another test is running
+    testRunnerLock.acquire();
+
     if (!execCommand(QStringLiteral("%1 install -r -g %2")
                         .arg(g_options.adbCommand, g_options.apkPath), nullptr, g_options.verbose)) {
         return 1;
@@ -678,5 +701,8 @@ int main(int argc, char *argv[])
     res &= execCommand(QStringLiteral("%1 uninstall %2").arg(g_options.adbCommand, g_options.package),
                        nullptr, g_options.verbose);
     fflush(stdout);
+
+    testRunnerLock.release();
+
     return res ? 0 : 1;
 }