diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index f2c55c321f..a7fbbf60b5 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -115,7 +115,8 @@ BOOL LLApp::sDisableCrashlogger = FALSE; BOOL LLApp::sLogInSignal = FALSE; // static -LLApp::EAppStatus LLApp::sStatus = LLApp::APP_STATUS_STOPPED; // Keeps track of application status +// Keeps track of application status +LLScalarCond LLApp::sStatus{LLApp::APP_STATUS_STOPPED}; LLAppErrorHandler LLApp::sErrorHandler = NULL; BOOL LLApp::sErrorThreadRunning = FALSE; @@ -582,7 +583,8 @@ static std::map statusDesc // static void LLApp::setStatus(EAppStatus status) { - sStatus = status; + // notify everyone waiting on sStatus any time its value changes + sStatus.set_all(status); // This can also happen very late in the application lifecycle -- don't // resurrect a deleted LLSingleton @@ -612,6 +614,12 @@ void LLApp::setError() setStatus(APP_STATUS_ERROR); } +// static +bool LLApp::sleep(F32Milliseconds duration) +{ + return ! sStatus.wait_for_unequal(duration, APP_STATUS_RUNNING); +} + void LLApp::setMiniDumpDir(const std::string &path) { if (path.empty()) @@ -671,28 +679,28 @@ void LLApp::setStopped() // static bool LLApp::isStopped() { - return (APP_STATUS_STOPPED == sStatus); + return (APP_STATUS_STOPPED == sStatus.get()); } // static bool LLApp::isRunning() { - return (APP_STATUS_RUNNING == sStatus); + return (APP_STATUS_RUNNING == sStatus.get()); } // static bool LLApp::isError() { - return (APP_STATUS_ERROR == sStatus); + return (APP_STATUS_ERROR == sStatus.get()); } // static bool LLApp::isQuitting() { - return (APP_STATUS_QUITTING == sStatus); + return (APP_STATUS_QUITTING == sStatus.get()); } // static diff --git a/indra/llcommon/llapp.h b/indra/llcommon/llapp.h index 245c73e3a2..e95929b865 100644 --- a/indra/llcommon/llapp.h +++ b/indra/llcommon/llapp.h @@ -28,9 +28,11 @@ #define LL_LLAPP_H #include +#include "llcond.h" #include "llrun.h" #include "llsd.h" #include +#include // Forward declarations class LLErrorThread; class LLLiveFile; @@ -211,6 +213,36 @@ public: static bool isExiting(); // Either quitting or error (app is exiting, cleanly or not) static int getPid(); + // + // Sleep for specified time while still running + // + // For use by a coroutine or thread that performs some maintenance on a + // periodic basis. (See also LLEventTimer.) This method supports the + // pattern of an "infinite" loop that sleeps for some time, performs some + // action, then sleeps again. The trouble with literally sleeping a worker + // thread is that it could potentially sleep right through attempted + // application shutdown. This method avoids that by returning false as + // soon as the application status changes away from APP_STATUS_RUNNING + // (isRunning()). + // + // sleep() returns true if it sleeps undisturbed for the entire specified + // duration. The idea is that you can code 'while sleep(duration) ...', + // which will break the loop once shutdown begins. + // + // Since any time-based LLUnit should be implicitly convertible to + // F32Milliseconds, accept that specific type as a proxy. + static bool sleep(F32Milliseconds duration); + // Allow any duration defined in terms of . + // One can imagine a wonderfully general bidirectional conversion system + // between any type derived from LLUnits::LLUnit and + // any std::chrono::duration -- but that doesn't yet exist. + template + bool sleep(const std::chrono::duration& duration) + { + // wait_for_unequal() has the opposite bool return convention + return ! sStatus.wait_for_unequal(duration, APP_STATUS_RUNNING); + } + /** @name Error handling methods */ //@{ /** @@ -241,8 +273,8 @@ public: // Return the Google Breakpad minidump filename after a crash. char *getMiniDumpFilename() { return mMinidumpPath; } - std::string* getStaticDebugFile() { return &mStaticDebugFileName; } - std::string* getDynamicDebugFile() { return &mDynamicDebugFileName; } + std::string* getStaticDebugFile() { return &mStaticDebugFileName; } + std::string* getDynamicDebugFile() { return &mDynamicDebugFileName; } // Write out a Google Breakpad minidump file. void writeMiniDump(); @@ -266,7 +298,7 @@ public: protected: static void setStatus(EAppStatus status); // Use this to change the application status. - static EAppStatus sStatus; // Reflects current application status + static LLScalarCond sStatus; // Reflects current application status static BOOL sErrorThreadRunning; // Set while the error thread is running static BOOL sDisableCrashlogger; // Let the OS handle crashes for us. std::wstring mCrashReportPipeStr; //Name of pipe to use for crash reporting. diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 118c60d7a6..613431ee6a 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -27,7 +27,6 @@ #ifndef LL_LLTHREAD_H #define LL_LLTHREAD_H -#include "llapp.h" #include "llapr.h" #include "boost/intrusive_ptr.hpp" #include "llrefcount.h" diff --git a/indra/llcommon/lluuid.cpp b/indra/llcommon/lluuid.cpp index bcff0a8501..67eddb37a4 100644 --- a/indra/llcommon/lluuid.cpp +++ b/indra/llcommon/lluuid.cpp @@ -33,6 +33,7 @@ #include #endif +#include "llapp.h" #include "lldefs.h" #include "llerror.h" diff --git a/indra/llcrashlogger/llcrashlogger.cpp b/indra/llcrashlogger/llcrashlogger.cpp index e77e0e2f6f..76f1b496fc 100644 --- a/indra/llcrashlogger/llcrashlogger.cpp +++ b/indra/llcrashlogger/llcrashlogger.cpp @@ -725,7 +725,7 @@ bool LLCrashLogger::init() #if LL_WINDOWS Sleep(1000); #else - sleep(1); + ::sleep(1); #endif locked = mKeyMaster.checkMaster(); } diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 3454da9451..825f1c2f22 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -31,6 +31,7 @@ */ #include "linden_common.h" +#include "llapp.h" #include "llassettype.h" #include "lldir.h" #include @@ -62,6 +63,34 @@ LLDiskCache::LLDiskCache(const std::string cache_dir, // } +// WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must +// NOT touch any LLDiskCache data without introducing and locking a mutex! + +// Interaction through the filesystem itself should be safe. Let’s say thread +// A is accessing the cache file for reading/writing and thread B is trimming +// the cache. Let’s also assume using llifstream to open a file and +// boost::filesystem::remove are not atomic (which will be pretty much the +// case). + +// Now, A is trying to open the file using llifstream ctor. It does some +// checks if the file exists and whatever else it might be doing, but has not +// issued the call to the OS to actually open the file yet. Now B tries to +// delete the file: If the file has been already marked as in use by the OS, +// deleting the file will fail and B will continue with the next file. A can +// safely continue opening the file. If the file has not yet been marked as in +// use, B will delete the file. Now A actually wants to open it, operation +// will fail, subsequent check via llifstream.is_open will fail, asset will +// have to be re-requested. (Assuming here the viewer will actually handle +// this situation properly, that can also happen if there is a file containing +// garbage.) + +// Other situation: B is trimming the cache and A wants to read a file that is +// about to get deleted. boost::filesystem::remove does whatever it is doing +// before actually deleting the file. If A opens the file before the file is +// actually gone, the OS call from B to delete the file will fail since the OS +// will prevent this. B continues with the next file. If the file is already +// gone before A finally gets to open it, this operation will fail and the +// asset will have to be re-requested. void LLDiskCache::purge() { //if (mEnableCacheDebugInfo) @@ -364,26 +393,17 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) return total_file_size; } -// Regular disk cache cleanup -FSPurgeDiskCacheThread::FSPurgeDiskCacheThread() : +LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : LLThread("PurgeDiskCacheThread", nullptr) { } -void FSPurgeDiskCacheThread::run() +void LLPurgeDiskCacheThread::run() { - constexpr F64 CHECK_INTERVAL = 60; - mTimer.setTimerExpirySec(CHECK_INTERVAL); - mTimer.start(); + constexpr std::chrono::seconds CHECK_INTERVAL{60}; - do + while (LLApp::instance()->sleep(CHECK_INTERVAL)) { - if (mTimer.checkExpirationAndReset(CHECK_INTERVAL)) - { - LLDiskCache::instance().purge(); - } - - ms_sleep(100); - } while (!isQuitting()); + LLDiskCache::instance().purge(); + } } -// diff --git a/indra/llfilesystem/lldiskcache.h b/indra/llfilesystem/lldiskcache.h index 13570d1126..6d63523b49 100644 --- a/indra/llfilesystem/lldiskcache.h +++ b/indra/llfilesystem/lldiskcache.h @@ -126,6 +126,13 @@ class LLDiskCache : /** * Purge the oldest items in the cache so that the combined size of all files * is no bigger than mMaxSizeBytes. + * + * WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must + * NOT touch any LLDiskCache data without introducing and locking a mutex! + * + * Purging the disk cache involves nontrivial work on the viewer's + * filesystem. If called on the main thread, this causes a noticeable + * freeze. */ void purge(); @@ -191,17 +198,12 @@ class LLDiskCache : bool mEnableCacheDebugInfo; }; -// Regular disk cache cleanup -class FSPurgeDiskCacheThread : public LLThread +class LLPurgeDiskCacheThread : public LLThread { public: - FSPurgeDiskCacheThread(); + LLPurgeDiskCacheThread(); protected: void run() override; - -private: - LLTimer mTimer; }; -// #endif // _LLDISKCACHE diff --git a/indra/llmessage/message.cpp b/indra/llmessage/message.cpp index 9b5050af41..a9d03c425c 100644 --- a/indra/llmessage/message.cpp +++ b/indra/llmessage/message.cpp @@ -46,6 +46,7 @@ #include "apr_poll.h" // linden library headers +#include "llapp.h" #include "indra_constants.h" #include "lldir.h" #include "llerror.h" diff --git a/indra/llplugin/llpluginprocessparent.cpp b/indra/llplugin/llpluginprocessparent.cpp index 6c6f819415..7088cf4d4b 100644 --- a/indra/llplugin/llpluginprocessparent.cpp +++ b/indra/llplugin/llpluginprocessparent.cpp @@ -28,6 +28,7 @@ #include "linden_common.h" +#include "llapp.h" #include "llpluginprocessparent.h" #include "llpluginmessagepipe.h" #include "llpluginmessageclasses.h" diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index f3105ef3e5..9963d4f096 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -735,7 +735,7 @@ LLAppViewer* LLAppViewer::sInstance = NULL; LLTextureCache* LLAppViewer::sTextureCache = NULL; LLImageDecodeThread* LLAppViewer::sImageDecodeThread = NULL; LLTextureFetch* LLAppViewer::sTextureFetch = NULL; -FSPurgeDiskCacheThread* LLAppViewer::sPurgeDiskCacheThread = NULL; // Regular disk cache cleanup +LLPurgeDiskCacheThread* LLAppViewer::sPurgeDiskCacheThread = NULL; std::string getRuntime() { @@ -2448,7 +2448,7 @@ bool LLAppViewer::cleanup() sTextureFetch->shutdown(); sTextureCache->shutdown(); sImageDecodeThread->shutdown(); - sPurgeDiskCacheThread->shutdown(); // Regular disk cache cleanup + sPurgeDiskCacheThread->shutdown(); sTextureFetch->shutDownTextureCacheThread() ; sTextureFetch->shutDownImageDecodeThread() ; @@ -2471,10 +2471,8 @@ bool LLAppViewer::cleanup() sImageDecodeThread = NULL; delete mFastTimerLogThread; mFastTimerLogThread = NULL; - // Regular disk cache cleanup delete sPurgeDiskCacheThread; sPurgeDiskCacheThread = NULL; - // if (LLFastTimerView::sAnalyzePerformance) { @@ -2599,7 +2597,7 @@ bool LLAppViewer::initThreads() sImageDecodeThread, enable_threads && true, app_metrics_qa_mode); - LLAppViewer::sPurgeDiskCacheThread = new FSPurgeDiskCacheThread(); // Regular disk cache cleanup + LLAppViewer::sPurgeDiskCacheThread = new LLPurgeDiskCacheThread(); if (LLTrace::BlockTimer::sLog || LLTrace::BlockTimer::sMetricLog) { @@ -5076,7 +5074,6 @@ bool LLAppViewer::initCache() LLDiskCache::getInstance()->purge(); } } - // Regular disk cache cleanup LLAppViewer::getPurgeDiskCacheThread()->start(); // FIRE-13066 diff --git a/indra/newview/llappviewer.h b/indra/newview/llappviewer.h index 9d6801c27d..ac4eb83d07 100644 --- a/indra/newview/llappviewer.h +++ b/indra/newview/llappviewer.h @@ -58,8 +58,8 @@ class LLImageDecodeThread; class LLTextureFetch; class LLWatchdogTimeout; class LLViewerJoystick; +class LLPurgeDiskCacheThread; class LLViewerRegion; // -class FSPurgeDiskCacheThread; // Regular disk cache cleanup extern LLTrace::BlockTimerStatHandle FTM_FRAME; @@ -119,7 +119,7 @@ public: static LLTextureCache* getTextureCache() { return sTextureCache; } static LLImageDecodeThread* getImageDecodeThread() { return sImageDecodeThread; } static LLTextureFetch* getTextureFetch() { return sTextureFetch; } - static FSPurgeDiskCacheThread* getPurgeDiskCacheThread() { return sPurgeDiskCacheThread; } // Regular disk cache cleanup + static LLPurgeDiskCacheThread* getPurgeDiskCacheThread() { return sPurgeDiskCacheThread; } static U32 getTextureCacheVersion() ; static U32 getObjectCacheVersion() ; @@ -309,7 +309,7 @@ private: static LLTextureCache* sTextureCache; static LLImageDecodeThread* sImageDecodeThread; static LLTextureFetch* sTextureFetch; - static FSPurgeDiskCacheThread* sPurgeDiskCacheThread; // Regular disk cache cleanup + static LLPurgeDiskCacheThread* sPurgeDiskCacheThread; S32 mNumSessions;