diff --git a/include/mega/gfx/isolatedprocess.h b/include/mega/gfx/isolatedprocess.h index bc498c7c83..d1caa711bb 100644 --- a/include/mega/gfx/isolatedprocess.h +++ b/include/mega/gfx/isolatedprocess.h @@ -28,22 +28,20 @@ class CancellableSleeper bool mCancelled = false; }; -class AutoStartLauncher +class AutoStartLauncher: public std::enable_shared_from_this { public: AutoStartLauncher(const std::vector& argv, std::function shutdowner); - ~AutoStartLauncher(); + bool start(); - void shutDownOnce(); + void stop(); private: bool startUntilSuccess(Process& process); - bool startLaunchLoopThread(); - - void exitLaunchLoopThread(); + bool exitLaunchLoopThread(); std::vector 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 mLauncher; HelloBeater mBeater; }; diff --git a/src/gfx/isolatedprocess.cpp b/src/gfx/isolatedprocess.cpp index bfb3579e57..647e7414d4 100644 --- a/src/gfx/isolatedprocess.cpp +++ b/src/gfx/isolatedprocess.cpp @@ -161,17 +161,6 @@ AutoStartLauncher::AutoStartLauncher(const std::vector& 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(400); + 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 @@ -222,7 +214,8 @@ bool AutoStartLauncher::startLaunchLoopThread() // if less than threshhold, it fails right after startup. if ((used < fastFailureThreshold) && !mShuttingDown) { - LOG_err << "process existed too fast: " << used.count() << " backoff" << backOff.count() << "ms"; + // LOG_verbose << "process existed too fast: " << used.count() << " backoff " + // << backOff.count() << "ms"; mSleeper.sleep(backOff); backOff = std::min(backOff * 2, maxBackoff); // double it and maxBackoff at most } @@ -233,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](){ @@ -241,11 +238,13 @@ bool AutoStartLauncher::startLaunchLoopThread() if (startUntilSuccess(process)) { bool ret = process.wait(); - LOG_debug << "wait: " << ret - << " hasSignal: " << process.hasTerminateBySignal() - << " " << (process.hasTerminateBySignal() ? std::to_string(process.getTerminatingSignal()) : "") - << " hasExited: " << process.hasExited() - << " " << (process.hasExited() ? std::to_string(process.getExitCode()) : ""); + LOG_verbose << "wait: " << ret << " hasSignal: " << process.hasTerminateBySignal() + << " " + << (process.hasTerminateBySignal() ? + std::to_string(process.getTerminatingSignal()) : + "") + << " hasExited: " << process.hasExited() << " " + << (process.hasExited() ? std::to_string(process.getExitCode()) : ""); } }); @@ -262,19 +261,33 @@ bool AutoStartLauncher::startLaunchLoopThread() // is just starting. so we'll retry in the loop, but there is no reason it // couldn't be shut down in 15 seconds // -void AutoStartLauncher::exitLaunchLoopThread() +// @return true if thread exits, otherwise false +bool AutoStartLauncher::exitLaunchLoopThread() { - milliseconds backOff(10); - while (mThreadIsRunning && backOff < 15s) + milliseconds interval{10}; + milliseconds totalWaitTime{0}; + while (mThreadIsRunning && totalWaitTime < 15s) { + LOG_verbose << "interval " << interval.count() << " totalWaitTime " + << totalWaitTime.count(); + // shutdown the started process if (mShutdowner) mShutdowner(); - std::this_thread::sleep_for(backOff); - backOff += 10ms; + + // wait + std::this_thread::sleep_for(interval); + + // Update total wait time + totalWaitTime += interval; + + // backoff + interval += 10ms; } + + return !mThreadIsRunning; } -void AutoStartLauncher::shutDownOnce() +void AutoStartLauncher::stop() { bool wasShuttingdown = mShuttingDown.exchange(true); if (wasShuttingdown) @@ -287,17 +300,23 @@ void AutoStartLauncher::shutDownOnce() // cancel sleeper, thread in sleep is woken up if it is mSleeper.cancel(); - exitLaunchLoopThread(); - if (mThread.joinable()) mThread.join(); + if (exitLaunchLoopThread()) + { + if (mThread.joinable()) + mThread.join(); + } + 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 l(mMutex); @@ -345,12 +364,22 @@ std::vector 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 diff --git a/src/process.cpp b/src/process.cpp index 9436ae9e5b..05f45a4811 100644 --- a/src/process.cpp +++ b/src/process.cpp @@ -205,21 +205,21 @@ bool Process::run(const vector& args, const unordered_map argv; for (vector::const_iterator i = args.begin(); i != args.end(); ++i) @@ -237,7 +237,7 @@ bool Process::run(const vector& args, const unordered_map parent process diff --git a/tests/integration/SdkTest_test.cpp b/tests/integration/SdkTest_test.cpp index f3eeb0665c..83dba0751f 100644 --- a/tests/integration/SdkTest_test.cpp +++ b/tests/integration/SdkTest_test.cpp @@ -265,28 +265,30 @@ namespace #endif } - MegaApiTest* newMegaApi(const char* appKey, - const char* basePath, - const char* userAgent, - unsigned workerThreadCount, - const int clientType = MegaApi::CLIENT_TYPE_DEFAULT) + MegaApiTestPointer newMegaApi(const char* appKey, + const char* basePath, + const char* userAgent, + unsigned workerThreadCount, + const int clientType = MegaApi::CLIENT_TYPE_DEFAULT) { - #ifdef ENABLE_ISOLATED_GFX +#ifdef ENABLE_ISOLATED_GFX const auto gfxworkerPath = sdk_test::getTestDataDir() / executableName("gfxworker"); const auto endpointName = newEndpointName(); std::unique_ptr provider{ MegaGfxProvider::createIsolatedInstance(endpointName.c_str(), gfxworkerPath.string().c_str()) }; - return new MegaApiTest(endpointName, - appKey, - provider.get(), - basePath, - userAgent, - workerThreadCount, - clientType); - #else - return new MegaApiTest(appKey, basePath, userAgent, workerThreadCount, clientType); - #endif + return MegaApiTestPointer{new MegaApiTest(appKey, + provider.get(), + basePath, + userAgent, + workerThreadCount, + clientType), + MegaApiTestDeleter{endpointName}}; +#else + return MegaApiTestPointer{ + new MegaApiTest(appKey, basePath, userAgent, workerThreadCount, clientType), + MegaApiTestDeleter{""}}; +#endif } enum class HasIcon @@ -346,23 +348,33 @@ MegaApiTest::MegaApiTest(const char* appKey, { } -MegaApiTest::MegaApiTest(const std::string& endpointName, - const char* appKey, +MegaApiTest::MegaApiTest(const char* appKey, MegaGfxProvider* provider, const char* basePath, const char* userAgent, unsigned workerThreadCount, const int clientType): - MegaApi(appKey, provider, basePath, userAgent, workerThreadCount, clientType), - mEndpointName(endpointName) + MegaApi(appKey, provider, basePath, userAgent, workerThreadCount, clientType) { } -MegaApiTest::~MegaApiTest() +MegaClient* MegaApiTest::getClient() +{ + return pImpl->getMegaClient(); +} + +void MegaApiTestDeleter::operator()(MegaApiTest* p) const { + delete p; + + // Clean up the socket file if it has been created and only after MegaApiTest is deleted. + // Reason: the GfxIsolatedProcess is desctructed in the subclass MegaApi + // Another alernative is to clean up the socket file in the GfxIsolatedProcess destructor. + // However it might clean up a socket file created by a new GfxIsolatedProcess is a same + // name is used alghouth it seems be rare. #if !defined(WIN32) && defined(ENABLE_ISOLATED_GFX) - // Clean up socket file if it has been created - if (mEndpointName.empty()) return; + if (mEndpointName.empty()) + return; if (std::error_code errorCode = SocketUtils::removeSocketFile(mEndpointName)) { @@ -371,12 +383,6 @@ MegaApiTest::~MegaApiTest() #endif } -MegaClient* MegaApiTest::getClient() -{ - return pImpl->getMegaClient(); -} - - void SdkTest::SetUp() { SdkTestBase::SetUp(); @@ -1614,11 +1620,11 @@ void SdkTest::configureTestInstance(unsigned index, ASSERT_FALSE(mApi[index].pwd.empty()) << "Set test account " << index << " password at the environment variable $" << passVarName; } - megaApi[index].reset(newMegaApi(APP_KEY.c_str(), - megaApiCacheFolder(static_cast(index)).c_str(), - USER_AGENT.c_str(), - unsigned(THREADS_PER_MEGACLIENT), - clientType)); + megaApi[index] = newMegaApi(APP_KEY.c_str(), + megaApiCacheFolder(static_cast(index)).c_str(), + USER_AGENT.c_str(), + unsigned(THREADS_PER_MEGACLIENT), + clientType); mApi[index].megaApi = megaApi[index].get(); // helps with restoring logging after tests that fiddle with log level @@ -7132,7 +7138,10 @@ TEST_F(SdkTest, SdkTestCloudraidTransfers) exitresumecount += 1; WaitMillisec(100); - megaApi[0].reset(newMegaApi(APP_KEY.c_str(), megaApiCacheFolder(0).c_str(), USER_AGENT.c_str(), unsigned(THREADS_PER_MEGACLIENT))); + megaApi[0] = newMegaApi(APP_KEY.c_str(), + megaApiCacheFolder(0).c_str(), + USER_AGENT.c_str(), + unsigned(THREADS_PER_MEGACLIENT)); mApi[0].megaApi = megaApi[0].get(); megaApi[0]->addListener(this); megaApi[0]->setMaxDownloadSpeed(1024 * 1024); @@ -11692,7 +11701,7 @@ TEST_F(SdkTest, DISABLED_StressTestSDKInstancesOverWritableFoldersOverWritableFo std::vector> trackers; trackers.resize(howMany); - std::vector> exportedFolderApis; + std::vector exportedFolderApis; exportedFolderApis.resize(howMany); std::vector exportedLinks; @@ -11732,11 +11741,11 @@ TEST_F(SdkTest, DISABLED_StressTestSDKInstancesOverWritableFoldersOverWritableFo // create apis to exported folders for (unsigned index = 0; index < howMany; index++) { - exportedFolderApis[static_cast(index)].reset( + exportedFolderApis[static_cast(index)] = newMegaApi(APP_KEY.c_str(), megaApiCacheFolder(static_cast(index) + 10).c_str(), USER_AGENT.c_str(), - static_cast(THREADS_PER_MEGACLIENT))); + static_cast(THREADS_PER_MEGACLIENT)); // reduce log level to something beareable exportedFolderApis[static_cast(index)]->setLogLevel(MegaApi::LOG_LEVEL_WARNING); @@ -11837,7 +11846,7 @@ TEST_F(SdkTest, WritableFolderSessionResumption) std::vector> trackers; trackers.resize(howMany); - std::vector> exportedFolderApis; + std::vector exportedFolderApis; exportedFolderApis.resize(howMany); std::vector exportedLinks; @@ -11883,11 +11892,11 @@ TEST_F(SdkTest, WritableFolderSessionResumption) // create apis to exported folders for (unsigned index = 0 ; index < howMany; index++ ) { - exportedFolderApis[index].reset( + exportedFolderApis[index] = newMegaApi(APP_KEY.c_str(), megaApiCacheFolder(static_cast(index) + 10).c_str(), USER_AGENT.c_str(), - static_cast(THREADS_PER_MEGACLIENT))); + static_cast(THREADS_PER_MEGACLIENT)); // reduce log level to something beareable exportedFolderApis[index]->setLogLevel(MegaApi::LOG_LEVEL_WARNING); diff --git a/tests/integration/SdkTest_test.h b/tests/integration/SdkTest_test.h index e78fb1b8fc..d4ee139714 100644 --- a/tests/integration/SdkTest_test.h +++ b/tests/integration/SdkTest_test.h @@ -249,23 +249,33 @@ class MegaApiTest: public MegaApi unsigned workerThreadCount = 1, const int clientType = MegaApi::CLIENT_TYPE_DEFAULT); - MegaApiTest(const std::string& endpointName, - const char* appKey, + MegaApiTest(const char* appKey, MegaGfxProvider* provider, const char* basePath = nullptr, const char* userAgent = nullptr, unsigned workerThreadCount = 1, const int clientType = MegaApi::CLIENT_TYPE_DEFAULT); - ~MegaApiTest(); - MegaClient* getClient(); +}; + +class MegaApiTestDeleter +{ +public: + MegaApiTestDeleter(const std::string& endpointName): + mEndpointName{endpointName} {}; + + MegaApiTestDeleter(): + MegaApiTestDeleter(""){}; + + void operator()(MegaApiTest* p) const; private: - // the endpoint name for isolated gfx std::string mEndpointName; }; +using MegaApiTestPointer = std::unique_ptr; + // Fixture class with common code for most of tests class SdkTest : public SdkTestBase, public MegaListener, public MegaRequestListener, MegaTransferListener, MegaLogger { @@ -417,7 +427,7 @@ class SdkTest : public SdkTestBase, public MegaListener, public MegaRequestListe }; std::vector mApi; - std::vector> megaApi; + std::vector megaApi; m_off_t onTransferStart_progress; m_off_t onTransferUpdate_progress;