Wait till all processes finished, ask for killing

See https://github.com/Martchus/syncthingtray/issues/15

connector/syncthingprocess.cpp

@@ -10,10 +10,13 @@ SyncthingProcess::SyncthingProcess(QObject *parent)
     : QProcess(parent)
     , m_manuallyStopped(false)
 {
+    m_killTimer.setInterval(3000);
+    m_killTimer.setSingleShot(true);
     setProcessChannelMode(QProcess::MergedChannels);
     connect(this, &SyncthingProcess::started, this, &SyncthingProcess::handleStarted);
     connect(this, static_cast<void (SyncthingProcess::*)(int exitCode, QProcess::ExitStatus exitStatus)>(&SyncthingProcess::finished), this,
         &SyncthingProcess::handleFinished);
+    connect(&m_killTimer, &QTimer::timeout, this, &SyncthingProcess::confirmKill);
 }
 
 void SyncthingProcess::restartSyncthing(const QString &cmd)
@@ -22,11 +25,9 @@ void SyncthingProcess::restartSyncthing(const QString &cmd)
         startSyncthing(cmd);
         return;
     }
-
     m_cmd = cmd;
     m_manuallyStopped = true;
-    // give Syncthing 5 seconds to terminate, otherwise kill it
+    m_killTimer.start();
-    QTimer::singleShot(5000, this, &SyncthingProcess::killToRestart);
     terminate();
 }
 
@@ -36,6 +37,7 @@ void SyncthingProcess::startSyncthing(const QString &cmd)
         return;
     }
     m_manuallyStopped = false;
+    m_killTimer.stop();
     if (cmd.isEmpty()) {
         start(QProcess::ReadOnly);
     } else {
@@ -49,11 +51,20 @@ void SyncthingProcess::stopSyncthing()
         return;
     }
     m_manuallyStopped = true;
-    // give Syncthing 5 seconds to terminate, otherwise kill it
+    m_killTimer.start();
-    QTimer::singleShot(5000, this, &SyncthingProcess::kill);
     terminate();
 }
 
+void SyncthingProcess::killSyncthing()
+{
+    if (!isRunning()) {
+        return;
+    }
+    m_manuallyStopped = true;
+    m_killTimer.stop();
+    kill();
+}
+
 void SyncthingProcess::handleStarted()
 {
     m_activeSince = DateTime::gmtNow();
@@ -64,6 +75,7 @@ void SyncthingProcess::handleFinished(int exitCode, QProcess::ExitStatus exitSta
     Q_UNUSED(exitCode)
     Q_UNUSED(exitStatus)
     m_activeSince = DateTime();
+    m_killTimer.stop();
     if (!m_cmd.isEmpty()) {
         startSyncthing(m_cmd);
         m_cmd.clear();

connector/syncthingprocess.h

@@ -6,6 +6,7 @@
 #include <c++utilities/chrono/datetime.h>
 
 #include <QProcess>
+#include <QTimer>
 
 namespace Data {
 
@@ -22,10 +23,14 @@ public:
     bool isActiveFor(unsigned int atLeastSeconds) const;
     bool isManuallyStopped() const;
 
+Q_SIGNALS:
+    void confirmKill();
+
 public Q_SLOTS:
     void restartSyncthing(const QString &cmd);
     void startSyncthing(const QString &cmd);
     void stopSyncthing();
+    void killSyncthing();
 
 private Q_SLOTS:
     void handleStarted();
@@ -35,6 +40,7 @@ private Q_SLOTS:
 private:
     QString m_cmd;
     ChronoUtilities::DateTime m_activeSince;
+    QTimer m_killTimer;
     bool m_manuallyStopped;
 };
 

scripts/dummy.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+function handle_int {
+    echo "Received SIGINT or SIGTERM, keep running for 15 seconds nevertheless"
+    sleep 15
+    exit -5
+}
+trap "handle_int" SIGINT SIGTERM
+
+while [[ true ]]; do
+    echo $RANDOM
+    sleep 1
+done

widgets/CMakeLists.txt

@@ -21,6 +21,7 @@ set(WIDGETS_HEADER_FILES
     misc/dbusstatusnotifier.h
     misc/internalerror.h
     misc/otherdialogs.h
+    misc/syncthingkiller.h
 )
 set(WIDGETS_SRC_FILES
     settings/settings.cpp
@@ -35,6 +36,7 @@ set(WIDGETS_SRC_FILES
     misc/dbusstatusnotifier.cpp
     misc/internalerror.cpp
     misc/otherdialogs.cpp
+    misc/syncthingkiller.cpp
 )
 set(RES_FILES
     resources/${META_PROJECT_NAME}icons.qrc

widgets/misc/syncthingkiller.cpp

@@ -0,0 +1,60 @@
+#include "./syncthingkiller.h"
+
+#include "../../connector/syncthingprocess.h"
+#define SYNCTHINGTESTHELPER_FOR_CLI
+#include "../../testhelper/helper.h"
+
+#include <QCoreApplication>
+#include <QMessageBox>
+
+using namespace std;
+using namespace Data;
+using namespace TestUtilities;
+
+namespace QtGui {
+
+SyncthingKiller::SyncthingKiller(std::vector<SyncthingProcess *> &&processes)
+    : m_processes(processes)
+{
+    for (auto *process : m_processes) {
+        process->stopSyncthing();
+        connect(process, &SyncthingProcess::confirmKill, this, &SyncthingKiller::confirmKill);
+    }
+}
+
+void SyncthingKiller::waitForFinished()
+{
+    for (auto *process : m_processes) {
+        if (!process->isRunning()) {
+            continue;
+        }
+        if (!waitForSignalsOrFail(noop, 0, signalInfo(this, &SyncthingKiller::ignored),
+                signalInfo(process, static_cast<void (SyncthingProcess::*)(int)>(&SyncthingProcess::finished)))) {
+            return;
+        }
+    }
+}
+
+void SyncthingKiller::confirmKill() const
+{
+    auto *const process = static_cast<SyncthingProcess *>(sender());
+    if (!process->isRunning()) {
+        return;
+    }
+
+    const auto msg(tr("The process %1 (PID: %2) has been requested to terminate but hasn't reacted yet. "
+                      "Kill the process?\n\n"
+                      "This dialog closes automatically when the process finally terminates.")
+                       .arg(process->program(), QString::number(process->processId())));
+    auto *const msgBox = new QMessageBox(QMessageBox::Critical, QCoreApplication::applicationName(), msg);
+    msgBox->setAttribute(Qt::WA_DeleteOnClose);
+    msgBox->addButton(tr("Keep running"), QMessageBox::RejectRole);
+    msgBox->addButton(tr("Kill process"), QMessageBox::AcceptRole);
+    connect(process, static_cast<void (QProcess::*)(int)>(&SyncthingProcess::finished), msgBox, &QMessageBox::close);
+    connect(msgBox, &QMessageBox::accepted, process, &SyncthingProcess::killSyncthing);
+    // FIXME: can not really ignore, just keep the process running
+    //connect(msgBox, &QMessageBox::rejected, this, &SyncthingKiller::ignored);
+    msgBox->show();
+}
+
+} // namespace QtGui

widgets/misc/syncthingkiller.h

@@ -0,0 +1,36 @@
+#ifndef SYNCTHINGWIDGETS_SYNCTHINGKILLER_H
+#define SYNCTHINGWIDGETS_SYNCTHINGKILLER_H
+
+#include "../global.h"
+
+#include <QObject>
+
+#include <vector>
+
+namespace Data {
+class SyncthingProcess;
+}
+
+namespace QtGui {
+
+class SYNCTHINGWIDGETS_EXPORT SyncthingKiller : public QObject {
+    Q_OBJECT
+public:
+    SyncthingKiller(std::vector<Data::SyncthingProcess *> &&processes);
+
+Q_SIGNALS:
+    void ignored();
+
+public Q_SLOTS:
+    void waitForFinished();
+
+private Q_SLOTS:
+    void confirmKill() const;
+
+private:
+    std::vector<Data::SyncthingProcess *> m_processes;
+};
+
+} // namespace QtGui
+
+#endif // SYNCTHINGWIDGETS_SYNCTHINGKILLER_H

widgets/settings/settings.cpp

@@ -1,6 +1,7 @@
 #include "./settings.h"
 #include "../../connector/syncthingnotifier.h"
 #include "../../connector/syncthingprocess.h"
+#include "../misc/syncthingkiller.h"
 #ifdef LIB_SYNCTHING_CONNECTOR_SUPPORT_SYSTEMD
 #include "../../connector/syncthingconnection.h"
 #include "../../connector/syncthingservice.h"
@@ -67,6 +68,17 @@ SyncthingProcess &Launcher::toolProcess(const QString &tool)
     return toolProcesses[tool];
 }
 
+std::vector<SyncthingProcess *> Launcher::allProcesses()
+{
+    vector<SyncthingProcess *> processes;
+    processes.reserve(1 + toolProcesses.size());
+    processes.push_back(&syncthingProcess());
+    for (auto &process : toolProcesses) {
+        processes.push_back(&process.second);
+    }
+    return processes;
+}
+
 /*!
  * \brief Starts all processes (Syncthing and tools) if autostart is enabled.
  */
@@ -83,16 +95,13 @@ void Launcher::autostart() const
     }
 }
 
+/*!
+ * \brief Terminates all launched processes.
+ * \remarks Waits until all processes have terminated. If a process hangs, the user is asked to kill.
+ */
 void Launcher::terminate()
 {
-    syncthingProcess().stopSyncthing();
+    QtGui::SyncthingKiller(allProcesses()).waitForFinished();
-    for (auto &process : toolProcesses) {
-        process.second.stopSyncthing();
-    }
-    syncthingProcess().waitForFinished();
-    for (auto &process : toolProcesses) {
-        process.second.waitForFinished();
-    }
 }
 
 Settings &values()

widgets/settings/settings.h

@@ -71,6 +71,7 @@ struct SYNCTHINGWIDGETS_EXPORT Launcher {
     QString syncthingCmd() const;
     QString toolCmd(const QString &tool) const;
     static Data::SyncthingProcess &toolProcess(const QString &tool);
+    static std::vector<Data::SyncthingProcess *> allProcesses();
     void autostart() const;
     static void terminate();
 };

widgets/settings/settingsdialog.cpp

@@ -728,6 +728,7 @@ void LauncherOptionPage::launch()
     }
     ui()->launchNowPushButton->hide();
     ui()->stopPushButton->show();
+    ui()->stopPushButton->setText(QCoreApplication::translate("QtGui::LauncherOptionPage", "Stop launched instance"));
     m_kill = false;
     if (m_tool.isEmpty()) {
         m_process.startSyncthing(values().launcher.syncthingCmd());
@@ -744,6 +745,7 @@ void LauncherOptionPage::stop()
     if (m_kill) {
         m_process.kill();
     } else {
+        ui()->stopPushButton->setText(QCoreApplication::translate("QtGui::LauncherOptionPage", "Kill launched instance"));
         m_kill = true;
         m_process.terminate();
     }

widgets/translations/syncthingwidgets_de_DE.ts

@@ -401,6 +401,7 @@
     </message>
     <message>
         <location filename="../settings/launcheroptionpage.ui" line="120"/>
+        <location filename="../settings/settingsdialog.cpp" line="731"/>
         <source>Stop launched instance</source>
         <translation>Stoppen</translation>
     </message>
@@ -446,6 +447,11 @@
 </source>
         <translation>%1 ist mit dem Statuscode %2 abgestürzt</translation>
     </message>
+    <message>
+        <location filename="../settings/settingsdialog.cpp" line="748"/>
+        <source>Kill launched instance</source>
+        <translation>Töten</translation>
+    </message>
 </context>
 <context>
     <name>QtGui::NotificationsOptionPage</name>
@@ -546,22 +552,22 @@
 <context>
     <name>QtGui::SettingsDialog</name>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="907"/>
+        <location filename="../settings/settingsdialog.cpp" line="909"/>
         <source>Tray</source>
         <translation></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="914"/>
+        <location filename="../settings/settingsdialog.cpp" line="916"/>
         <source>Web view</source>
         <translation>Weboberfläche</translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="921"/>
+        <location filename="../settings/settingsdialog.cpp" line="923"/>
         <source>Startup</source>
         <translation>Starten</translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="943"/>
+        <location filename="../settings/settingsdialog.cpp" line="945"/>
         <source>Settings</source>
         <translation>Einstellungen</translation>
     </message>
@@ -642,6 +648,28 @@
         <translation>Nicht mit anderen Geräten verbunden</translation>
     </message>
 </context>
+<context>
+    <name>QtGui::SyncthingKiller</name>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="45"/>
+        <source>The process %1 (PID: %2) has been requested to terminate but hasn&apos;t reacted yet. Kill the process?
+
+This dialog closes automatically when the process finally terminates.</source>
+        <translation>Der Prozess %1 (PID: %2) wurde aufgefordert sich zu beenden, reagiert aber nicht. Soll der Prozess getötet werden?
+
+Dieser Dialog schließt sich automatisch, wenn der Prozess beendet wird.</translation>
+    </message>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="51"/>
+        <source>Keep running</source>
+        <translation>Prozess laufen lassen</translation>
+    </message>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="52"/>
+        <source>Kill process</source>
+        <translation>Prozess töten</translation>
+    </message>
+</context>
 <context>
     <name>QtGui::SystemdOptionPage</name>
     <message>
@@ -678,8 +706,8 @@
         <location filename="../settings/systemdoptionpage.ui" line="99"/>
         <location filename="../settings/systemdoptionpage.ui" line="154"/>
         <location filename="../settings/systemdoptionpage.ui" line="231"/>
-        <location filename="../settings/settingsdialog.cpp" line="828"/>
+        <location filename="../settings/settingsdialog.cpp" line="830"/>
-        <location filename="../settings/settingsdialog.cpp" line="840"/>
+        <location filename="../settings/settingsdialog.cpp" line="842"/>
         <source>unknown</source>
         <translation>unbekannt</translation>
     </message>
@@ -713,12 +741,12 @@
         <translation>Stoppen</translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="801"/>
+        <location filename="../settings/settingsdialog.cpp" line="803"/>
         <source>specified unit is either inactive or doesn&apos;t exist</source>
         <translation>angegebene Unit ist entweder nicht geladen oder existiert nicht</translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="823"/>
+        <location filename="../settings/settingsdialog.cpp" line="825"/>
         <source>since </source>
         <translation>seit </translation>
     </message>
@@ -785,7 +813,7 @@
     <name>QtGui::WebViewOptionPage</name>
     <message>
         <location filename="../settings/webviewoptionpage.ui" line="6"/>
-        <location filename="../settings/settingsdialog.cpp" line="862"/>
+        <location filename="../settings/settingsdialog.cpp" line="864"/>
         <source>General</source>
         <translation>Allgemein</translation>
     </message>
@@ -815,7 +843,7 @@
         <translation>Lasse Weboberfläche im Hintgergrund weiter offen, wenn Fenster nicht offen</translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="864"/>
+        <location filename="../settings/settingsdialog.cpp" line="866"/>
         <source>Syncthing Tray has not been built with vieb view support utilizing either Qt WebKit or Qt WebEngine.
 The Web UI will be opened in the default web browser instead.</source>
         <translation>Syncthing Tray wurde nicht mit Unterstützung für die eingebaute Anzeige der Weboberfläche unter Verwendung von Qt WebKit oder Qt WebEngine gebaut.
@@ -825,7 +853,7 @@ Die Weboberfläche wird stattdessen im Standardwebrowser geöffnet.</translation
 <context>
     <name>Settings::restore</name>
     <message>
-        <location filename="../settings/settings.cpp" line="150"/>
+        <location filename="../settings/settings.cpp" line="159"/>
         <source>Unable to load certificate &quot;%1&quot; when restoring settings.</source>
         <translation>Fehler beim laden des Zertifikats &quot;%1&quot; beim wiederherstellen der Einstellungen.</translation>
     </message>

widgets/translations/syncthingwidgets_en_US.ts

@@ -401,6 +401,7 @@
     </message>
     <message>
         <location filename="../settings/launcheroptionpage.ui" line="120"/>
+        <location filename="../settings/settingsdialog.cpp" line="731"/>
         <source>Stop launched instance</source>
         <translation type="unfinished"></translation>
     </message>
@@ -446,6 +447,11 @@
 </source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <location filename="../settings/settingsdialog.cpp" line="748"/>
+        <source>Kill launched instance</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>QtGui::NotificationsOptionPage</name>
@@ -546,22 +552,22 @@
 <context>
     <name>QtGui::SettingsDialog</name>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="907"/>
+        <location filename="../settings/settingsdialog.cpp" line="909"/>
         <source>Tray</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="914"/>
+        <location filename="../settings/settingsdialog.cpp" line="916"/>
         <source>Web view</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="921"/>
+        <location filename="../settings/settingsdialog.cpp" line="923"/>
         <source>Startup</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="943"/>
+        <location filename="../settings/settingsdialog.cpp" line="945"/>
         <source>Settings</source>
         <translation type="unfinished"></translation>
     </message>
@@ -642,6 +648,26 @@
         <translation type="unfinished"></translation>
     </message>
 </context>
+<context>
+    <name>QtGui::SyncthingKiller</name>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="45"/>
+        <source>The process %1 (PID: %2) has been requested to terminate but hasn&apos;t reacted yet. Kill the process?
+
+This dialog closes automatically when the process finally terminates.</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="51"/>
+        <source>Keep running</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <location filename="../misc/syncthingkiller.cpp" line="52"/>
+        <source>Kill process</source>
+        <translation type="unfinished"></translation>
+    </message>
+</context>
 <context>
     <name>QtGui::SystemdOptionPage</name>
     <message>
@@ -675,8 +701,8 @@
         <location filename="../settings/systemdoptionpage.ui" line="99"/>
         <location filename="../settings/systemdoptionpage.ui" line="154"/>
         <location filename="../settings/systemdoptionpage.ui" line="231"/>
-        <location filename="../settings/settingsdialog.cpp" line="828"/>
+        <location filename="../settings/settingsdialog.cpp" line="830"/>
-        <location filename="../settings/settingsdialog.cpp" line="840"/>
+        <location filename="../settings/settingsdialog.cpp" line="842"/>
         <source>unknown</source>
         <translation type="unfinished"></translation>
     </message>
@@ -711,12 +737,12 @@
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="801"/>
+        <location filename="../settings/settingsdialog.cpp" line="803"/>
         <source>specified unit is either inactive or doesn&apos;t exist</source>
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="823"/>
+        <location filename="../settings/settingsdialog.cpp" line="825"/>
         <source>since </source>
         <translation type="unfinished"></translation>
     </message>
@@ -783,7 +809,7 @@
     <name>QtGui::WebViewOptionPage</name>
     <message>
         <location filename="../settings/webviewoptionpage.ui" line="6"/>
-        <location filename="../settings/settingsdialog.cpp" line="862"/>
+        <location filename="../settings/settingsdialog.cpp" line="864"/>
         <source>General</source>
         <translation type="unfinished"></translation>
     </message>
@@ -813,7 +839,7 @@
         <translation type="unfinished"></translation>
     </message>
     <message>
-        <location filename="../settings/settingsdialog.cpp" line="864"/>
+        <location filename="../settings/settingsdialog.cpp" line="866"/>
         <source>Syncthing Tray has not been built with vieb view support utilizing either Qt WebKit or Qt WebEngine.
 The Web UI will be opened in the default web browser instead.</source>
         <translation type="unfinished"></translation>
@@ -822,7 +848,7 @@ The Web UI will be opened in the default web browser instead.</source>
 <context>
     <name>Settings::restore</name>
     <message>
-        <location filename="../settings/settings.cpp" line="150"/>
+        <location filename="../settings/settings.cpp" line="159"/>
         <source>Unable to load certificate &quot;%1&quot; when restoring settings.</source>
         <translation type="unfinished"></translation>
     </message>