Merge pull request #942 from isanae/lock-fixes

Lock fixes

8 files changed, 164 insertions, 41 deletions

diff --git a/src/main.cpp b/src/main.cpp
index eeabd497..29d2d02c 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -745,7 +745,6 @@ int runApplication(MOApplication &application, SingleInstance &instance,
       splash.finish(&mainWindow);
 
       res = application.exec();
-      mainWindow.onBeforeClose();
       mainWindow.close();
 
       NexusInterface::instance(&pluginContainer)
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index a29ea8ab..284c33e3 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1306,6 +1306,21 @@ void MainWindow::onBeforeClose()
 
 void MainWindow::closeEvent(QCloseEvent* event)
 {
+  if (isVisible()) {
+    // this is messy
+    //
+    // the main problem this is solving is when closing MO, then getting the
+    // lock overlay because processes are still running, then pressing the X
+    // again
+    //
+    // in this case, closeEvent() is _not_ called for the second event and the
+    // window is immediately hidden
+    //
+    // this always saves the settings here; in the event where a lock overlay
+    // is then shown, it might save settings multiple times, but it's harmless
+    onBeforeClose();
+  }
+
   // this happens for two reasons:
   //  1) the user requested to close the window, such as clicking the X
   //  2) close() is called in runApplication() after application.exec()
@@ -1317,18 +1332,32 @@ void MainWindow::closeEvent(QCloseEvent* event)
   //
   // for 2), the settings have been saved and the window can just close
 
-  if (ModOrganizerExiting()) {
+  if (ModOrganizerCanCloseNow()) {
     // the user has confirmed if necessary and all settings have been saved,
     // just close it
     QMainWindow::closeEvent(event);
-  } else {
-    // never close the window because settings might need to be changed
+    return;
+  }
+
+  if (UILocker::instance().locked()) {
+    // don't bother asking the user to confirm if the ui is already locked
     event->ignore();
+    ExitModOrganizer(Exit::Force);
+    return;
+  }
 
-    // start the process of exiting, which may require confirmation by calling
-    // canExit(), among other things
-    ExitModOrganizer();
+  if (ModOrganizerExiting()) {
+    // ignore repeated attempts
+    event->ignore();
+    return;
   }
+
+  // never close the window because settings might need to be changed
+  event->ignore();
+
+  // start the process of exiting, which may require confirmation by calling
+  // canExit(), among other things
+  ExitModOrganizer();
 }
 
 bool MainWindow::canExit()
diff --git a/src/processrunner.cpp b/src/processrunner.cpp
index 19aae632..945d61c3 100644
--- a/src/processrunner.cpp
+++ b/src/processrunner.cpp
@@ -225,7 +225,7 @@ const std::chrono::milliseconds Infinite(-1);
 //
 std::optional<ProcessRunner::Results> timedWait(
   HANDLE handle, DWORD pid, UILocker::Session& ls,
-  std::chrono::milliseconds wait)
+  std::chrono::milliseconds wait, std::atomic<bool>& interrupt)
 {
   using namespace std::chrono;
 
@@ -234,7 +234,7 @@ std::optional<ProcessRunner::Results> timedWait(
     start = high_resolution_clock::now();
   }
 
-  for (;;) {
+  while (!interrupt) {
     // wait for a very short while, allows for processing events below
     const auto r = singleWait(handle, pid);
 
@@ -286,10 +286,13 @@ std::optional<ProcessRunner::Results> timedWait(
       }
     }
   }
+
+  log::debug("waiting for {} interrupted", pid);
+  return ProcessRunner::ForceUnlocked;
 }
 
 ProcessRunner::Results waitForProcessesThreadImpl(
-  HANDLE job, UILocker::Session& ls)
+  HANDLE job, UILocker::Session& ls, std::atomic<bool>& interrupt)
 {
   using namespace std::chrono;
 
@@ -301,7 +304,7 @@ ProcessRunner::Results waitForProcessesThreadImpl(
   const milliseconds defaultWait(50);
   auto wait = defaultWait;
 
-  for (;;) {
+  while (!interrupt) {
     auto ip = getInterestingProcess(job);
     if (!ip.handle) {
       // nothing to wait on
@@ -325,7 +328,7 @@ ProcessRunner::Results waitForProcessesThreadImpl(
       wait = Infinite;
     }
 
-    const auto r = timedWait(ip.handle.get(), ip.p.pid(), ls, wait);
+    const auto r = timedWait(ip.handle.get(), ip.p.pid(), ls, wait, interrupt);
     if (r) {
       if (*r == ProcessRunner::Results::Completed) {
         // process completed, check another one, reset the wait time to find
@@ -341,12 +344,16 @@ ProcessRunner::Results waitForProcessesThreadImpl(
     // processes
     wait = std::min(wait * 2, milliseconds(2000));
   }
+
+  log::debug("waiting for processes interrupted");
+  return ProcessRunner::ForceUnlocked;
 }
 
 void waitForProcessesThread(
-  ProcessRunner::Results& result, HANDLE job, UILocker::Session& ls)
+  ProcessRunner::Results& result, HANDLE job, UILocker::Session& ls,
+  std::atomic<bool>& interrupt)
 {
-  result = waitForProcessesThreadImpl(job, ls);
+  result = waitForProcessesThreadImpl(job, ls, interrupt);
   ls.unlock();
 }
 
@@ -370,18 +377,23 @@ ProcessRunner::Results waitForProcesses(
     if (!::AssignProcessToJobObject(job.get(), h)) {
       const auto e = GetLastError();
 
-      log::error(
-        "can't assign process to job to wait for processes, {}",
-        formatSystemMessage(e));
+      // this happens when closing MO while multiple processes are running,
+      // so the logging is disabled until it gets fixed
+
+      //log::error(
+      //  "can't assign process to job to wait for processes, {}",
+      //  formatSystemMessage(e));
 
       // keep going
     }
   }
 
   auto results = ProcessRunner::Running;
+  std::atomic<bool> interrupt(false);
 
   auto* t = QThread::create(
-    waitForProcessesThread, std::ref(results), job.get(), std::ref(ls));
+    waitForProcessesThread,
+    std::ref(results), job.get(), std::ref(ls), std::ref(interrupt));
 
   QEventLoop events;
   QObject::connect(t, &QThread::finished, [&]{
@@ -391,6 +403,11 @@ ProcessRunner::Results waitForProcesses(
   t->start();
   events.exec();
 
+  if (t->isRunning()) {
+    interrupt = true;
+    t->wait();
+  }
+
   delete t;
 
   return results;
@@ -860,11 +877,12 @@ ProcessRunner::Results ProcessRunner::waitForAllUSVFSProcessesWithLock(
 
   auto r = Error;
 
-  withLock([&](auto& ls) {
-    for (;;) {
+  for (;;) {
+    withLock([&](auto& ls) {
       const auto processes = getRunningUSVFSProcesses();
       if (processes.empty()) {
-        break;
+        r = Completed;
+        return;
       }
 
       r = waitForProcesses(processes, ls);
@@ -875,10 +893,13 @@ ProcessRunner::Results ProcessRunner::waitForAllUSVFSProcessesWithLock(
       }
 
       // this process is completed, check for others
-    }
+      r = Running;
+    });
 
-    r = Completed;
-  });
+    if (r != Running) {
+      break;
+    }
+  }
 
   return r;
 }
diff --git a/src/shared/util.cpp b/src/shared/util.cpp
index 32eb825c..baceddeb 100644
--- a/src/shared/util.cpp
+++ b/src/shared/util.cpp
@@ -294,6 +294,7 @@ QString getUsvfsVersionString()
 

 

 static bool g_exiting = false;

+static bool g_canClose = false;

 

 MainWindow* findMainWindow()

 {

@@ -312,6 +313,9 @@ bool ExitModOrganizer(ExitFlags e)
     return true;

   }

 

+  g_exiting = true;

+  MOBase::Guard g([&]{ g_exiting = false; });

+

   if (!e.testFlag(Exit::Force)) {

     if (auto* mw=findMainWindow()) {

       if (!mw->canExit()) {

@@ -320,7 +324,7 @@ bool ExitModOrganizer(ExitFlags e)
     }

   }

 

-  g_exiting = true;

+  g_canClose = true;

 

   const int code = (e.testFlag(Exit::Restart) ? RestartExitCode : 0);

   qApp->exit(code);

@@ -328,6 +332,11 @@ bool ExitModOrganizer(ExitFlags e)
   return true;

 }

 

+bool ModOrganizerCanCloseNow()

+{

+  return g_canClose;

+}

+

 bool ModOrganizerExiting()

 {

   return g_exiting;

diff --git a/src/shared/util.h b/src/shared/util.h
index e87244b6..e8a58549 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -65,6 +65,7 @@ Q_DECLARE_OPERATORS_FOR_FLAGS(ExitFlags);
 

 bool ExitModOrganizer(ExitFlags e=Exit::Normal);

 bool ModOrganizerExiting();

+bool ModOrganizerCanCloseNow();

 void ResetExitFlag();

 

 #endif // UTIL_H

diff --git a/src/spawn.cpp b/src/spawn.cpp
index f95846c8..33bdbc05 100644
--- a/src/spawn.cpp
+++ b/src/spawn.cpp
@@ -498,14 +498,16 @@ DWORD spawn(const SpawnParameters& sp, HANDLE& processHandle)
   const auto wcommandLine = commandLine.toStdWString();

   const auto wcwd = cwd.toStdWString();

 

+  const DWORD flags = CREATE_BREAKAWAY_FROM_JOB;

+

   if (sp.hooked) {

     success = ::CreateProcessHooked(

       nullptr, const_cast<wchar_t*>(wcommandLine.c_str()), nullptr, nullptr,

-      inheritHandles, 0, nullptr, wcwd.c_str(), &si, &pi);

+      inheritHandles, flags, nullptr, wcwd.c_str(), &si, &pi);

   } else {

     success = ::CreateProcess(

       nullptr, const_cast<wchar_t*>(wcommandLine.c_str()), nullptr, nullptr,

-      inheritHandles, 0, nullptr, wcwd.c_str(), &si, &pi);

+      inheritHandles, flags, nullptr, wcwd.c_str(), &si, &pi);

   }

 

   const auto e = GetLastError();

diff --git a/src/uilocker.cpp b/src/uilocker.cpp
index 8c4e0c2c..07fe7f1b 100644
--- a/src/uilocker.cpp
+++ b/src/uilocker.cpp
@@ -20,6 +20,9 @@ public:
 
   ~UILockerInterface()
   {
+    if (m_topLevel) {
+      delete m_topLevel.data();
+    }
   }
 
   void checkTarget()
@@ -31,14 +34,8 @@ public:
 
   bool set()
   {
-    QWidget* newTarget = nullptr;
-
-    newTarget = m_mainUI;
-    if (auto* w = QApplication::activeModalWidget()) {
-      newTarget = w;
-    }
-
-    if (newTarget == m_target) {
+    QWidget* newTarget = findTarget();
+    if (m_topLevel && newTarget == m_target) {
       return false;
     }
 
@@ -98,8 +95,8 @@ public:
 
   QWidget* topLevel()
   {
-    return m_topLevel.get();
-  }
+    return m_topLevel.data();
+   }
 
 private:
   class Filter : public QObject
@@ -129,7 +126,7 @@ private:
   std::unique_ptr<QTimer> m_timer;
   QWidget* m_mainUI;
   QWidget* m_target;
-  std::unique_ptr<QWidget> m_topLevel;
+  QPointer<QWidget> m_topLevel;
   QLabel* m_message;
   QLabel* m_info;
   QStringList m_labels;
@@ -143,6 +140,55 @@ private:
     return (m_target != nullptr);
   }
 
+  QWidget* findTarget()
+  {
+    auto isValidTarget = [](QWidget* w) {
+      // skip message boxes
+      if (dynamic_cast<QMessageBox*>(w)) {
+        return false;
+      }
+
+      // skip invisible widgets
+      if (!w->isVisible()) {
+        return false;
+      }
+
+      // skip windows that are too small
+      if (w->height() < 150) {
+        return false;
+      }
+
+      return true;
+    };
+
+
+    // find a modal dialog
+    QWidget* w = QApplication::activeModalWidget();
+
+    while (w && w != m_mainUI) {
+      if (isValidTarget(w)) {
+        return w;
+      }
+
+      w = w->parentWidget();
+    }
+
+    // find a non-modal dialog that's a child of the main window
+    if (m_mainUI) {
+      const auto topLevels = QApplication::topLevelWidgets();
+
+      for (auto* w : topLevels) {
+        if (w && w->parentWidget() == m_mainUI) {
+          if (isValidTarget(w)) {
+            return w;
+          }
+        }
+      }
+    }
+
+    return m_mainUI;
+  }
+
   QWidget* createTransparentWidget(QWidget* parent=nullptr)
   {
     auto* w = new QWidget(parent);
@@ -156,7 +202,12 @@ private:
 
   QFrame* createOverlay(QWidget* mainUI)
   {
-    m_topLevel.reset(createTransparentWidget(mainUI));
+    if (m_topLevel) {
+      delete m_topLevel.data();
+      m_topLevel.clear();
+    }
+
+    m_topLevel = createTransparentWidget(mainUI);
     m_topLevel->setWindowFlags(m_topLevel->windowFlags() & Qt::FramelessWindowHint);
     m_topLevel->setGeometry(mainUI->rect());
 
@@ -171,7 +222,12 @@ private:
 
   QFrame* createDialog()
   {
-    m_topLevel.reset(new QDialog);
+    if (m_topLevel) {
+      delete m_topLevel.data();
+      m_topLevel.clear();
+    }
+
+    m_topLevel = new QDialog;
 
     return createFrame();
   }
@@ -195,7 +251,7 @@ private:
       ly->setContentsMargins(0, 0, 0, 0);
     }
 
-    auto* grid = new QGridLayout(m_topLevel.get());
+    auto* grid = new QGridLayout(m_topLevel.data());
     grid->addWidget(createTransparentWidget(), 0, 1);
     grid->addWidget(createTransparentWidget(), 2, 1);
     grid->addWidget(createTransparentWidget(), 1, 0);
@@ -405,6 +461,11 @@ std::shared_ptr<UILocker::Session> UILocker::lock(Reasons reason)
   return ls;
 }
 
+bool UILocker::locked() const
+{
+  return !m_sessions.empty();
+}
+
 void UILocker::unlock(Session* s)
 {
   auto itor = m_sessions.begin();
diff --git a/src/uilocker.h b/src/uilocker.h
index d8c22999..cc467184 100644
--- a/src/uilocker.h
+++ b/src/uilocker.h
@@ -70,6 +70,7 @@ public:
   void setUserInterface(QWidget* parent);
 
   std::shared_ptr<Session> lock(Reasons reason);
+  bool locked() const;
 
   Results result() const;