Allow AutoStartLaucher's thread be detached safely

meganz · Jan 6, 2025 · 95a7dbf · 95a7dbf
1 parent d68305b
commit 95a7dbf
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 34 deletions.
diff --git a/include/mega/gfx/isolatedprocess.h b/include/mega/gfx/isolatedprocess.h
@@ -28,21 +28,19 @@ class CancellableSleeper
     bool                    mCancelled = false;
 };
 
-class AutoStartLauncher
+class AutoStartLauncher: public std::enable_shared_from_this<AutoStartLauncher>
 {
 public:
     AutoStartLauncher(const std::vector<std::string>& argv, std::function<void()> shutdowner);
 
-    ~AutoStartLauncher();
+    bool start();
 
-    void shutDownOnce();
+    void stop();
 
 private:
 
     bool startUntilSuccess(Process& process);
 
-    bool startLaunchLoopThread();
-
     bool exitLaunchLoopThread();
 
     std::vector<std::string> mArgv;
@@ -137,12 +135,14 @@ class GfxIsolatedProcess
 
     GfxIsolatedProcess(const Params& params);
 
+    ~GfxIsolatedProcess();
+
     const std::string& endpointName() const { return mEndpointName; }
 private:
 
     std::string mEndpointName;
 
-    AutoStartLauncher mLauncher;
+    std::shared_ptr<AutoStartLauncher> mLauncher;
 
     HelloBeater mBeater;
 };

diff --git a/src/gfx/isolatedprocess.cpp b/src/gfx/isolatedprocess.cpp
@@ -161,17 +161,6 @@ AutoStartLauncher::AutoStartLauncher(const std::vector<std::string>& argv, std::
     mShutdowner(std::move(shutdowner))
 {
     assert(!mArgv.empty());
-
-    // preventive check: at least one element (executable)
-    if (!mArgv.empty())
-    {
-        // launch loop thread
-        startLaunchLoopThread();
-    }
-    else
-    {
-        LOG_fatal << "AutoStartLauncher argv is empty";
-    }
 }
 
 bool AutoStartLauncher::startUntilSuccess(Process& process)
@@ -201,11 +190,14 @@ bool AutoStartLauncher::startUntilSuccess(Process& process)
     return false;
 }
 
-bool AutoStartLauncher::startLaunchLoopThread()
+bool AutoStartLauncher::start()
 {
     static const milliseconds maxBackoff(3000);
     static const milliseconds fastFailureThreshold(1000);
 
+    if (mArgv.empty())
+        return false;
+
     // There are permanent startup failure such as missing DLL. This is not likey to happen
     // at customer's side as it will be installed properly. It is more likely during development
     // and testing phases. We want to implement some backOff to reduce CPU usage if it does happen
@@ -234,7 +226,11 @@ bool AutoStartLauncher::startLaunchLoopThread()
         }
     };
 
-    auto launcher = [this, backoffForFastFailure]() {
+    auto launcher = [this, backoffForFastFailure]()
+    {
+        // Keep a copy, so the object is always live while the code is running
+        auto keepRef = shared_from_this();
+
         mThreadIsRunning = true;
 
         backoffForFastFailure([this](){
@@ -272,7 +268,9 @@ bool AutoStartLauncher::exitLaunchLoopThread()
     milliseconds totalWaitTime{0};
     while (mThreadIsRunning && totalWaitTime < 15s)
     {
-        LOG_debug << "interval " << interval.count() << " totalWaitTime " << totalWaitTime.count();
+        LOG_verbose << "interval " << interval.count() << " totalWaitTime "
+                    << totalWaitTime.count();
+
         // shutdown the started process
         if (mShutdowner) mShutdowner();
 
@@ -289,7 +287,7 @@ bool AutoStartLauncher::exitLaunchLoopThread()
     return !mThreadIsRunning;
 }
 
-void AutoStartLauncher::shutDownOnce()
+void AutoStartLauncher::stop()
 {
     bool wasShuttingdown = mShuttingDown.exchange(true);
     if (wasShuttingdown)
@@ -310,18 +308,15 @@ void AutoStartLauncher::shutDownOnce()
     else
     {
         // Defensive: the thread doesn't exit, detach the thread
+        // We had such bug and it is usually a bug
+        // assert(false && "AutoStartLauncher detaching loop thread");
         LOG_warn << "AutoStartLauncher detaching loop thread";
         mThread.detach();
     }
 
     LOG_info << "AutoStartLauncher is down";
 }
 
-AutoStartLauncher::~AutoStartLauncher()
-{
-    shutDownOnce();
-}
-
 bool CancellableSleeper::sleep(const milliseconds& period)
 {
     std::unique_lock<std::mutex> l(mMutex);
@@ -369,12 +364,22 @@ std::vector<std::string> GfxIsolatedProcess::Params::toArgs() const
 // We divide keepAliveInSeconds by three to set up mBeater so that it allows at least two
 // beats within the keep-alive period.
 GfxIsolatedProcess::GfxIsolatedProcess(const Params& params):
-    mEndpointName(params.endpointName),
-    mLauncher(params.toArgs(),
-              [endpointName = params.endpointName]()
-              {
-                  shutdown(endpointName);
-              }),
-    mBeater(seconds(params.keepAliveInSeconds / 3), params.endpointName)
-{}
+    mEndpointName{
+        params.endpointName
+},
+    mLauncher{new AutoStartLauncher{params.toArgs(),
+                                    [endpointName = params.endpointName]()
+                                    {
+                                        shutdown(endpointName);
+                                    }}},
+    mBeater{seconds(params.keepAliveInSeconds / 3), params.endpointName}
+{
+    mLauncher->start();
 }
+
+GfxIsolatedProcess::~GfxIsolatedProcess()
+{
+    mLauncher->stop();
+}
+
+} // Namespace