From 168d177197bd7558bbe0ca13d01c984ad8638da7 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 14:39:51 -0800 Subject: This set of changes reverts the merge with master (git revert c83e740) and results in a version of the DRTVWR-519 that matches what was presemt before it was deployed as a release viewer *plus* 3 small fixes from Maxim (See commits). This branch can now be used for additional fixes before eventually being used to release D-519 as normal --- indra/llfilesystem/lldiskcache.cpp | 327 +++++++++++++++++++++++++++++++++++++ 1 file changed, 327 insertions(+) create mode 100644 indra/llfilesystem/lldiskcache.cpp (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp new file mode 100644 index 0000000000..c9f7684b5a --- /dev/null +++ b/indra/llfilesystem/lldiskcache.cpp @@ -0,0 +1,327 @@ +/** + * @file lldiskcache.cpp + * @brief The disk cache implementation. + * + * Note: Rather than keep the top level function comments up + * to date in both the source and header files, I elected to + * only have explicit comments about each function and variable + * in the header - look there for details. The same is true for + * description of how this code is supposed to work. + * + * $LicenseInfo:firstyear=2009&license=viewerlgpl$ + * Second Life Viewer Source Code + * Copyright (C) 2020, Linden Research, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; + * version 2.1 of the License only. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * Linden Research, Inc., 945 Battery Street, San Francisco, CA 94111 USA + * $/LicenseInfo$ + */ + +#include "linden_common.h" +#include "llassettype.h" +#include "lldir.h" +#include +#include +#include + +#include "lldiskcache.h" + +LLDiskCache::LLDiskCache(const std::string cache_dir, + const int max_size_bytes, + const bool enable_cache_debug_info) : + mCacheDir(cache_dir), + mMaxSizeBytes(max_size_bytes), + mEnableCacheDebugInfo(enable_cache_debug_info) +{ + mCacheFilenamePrefix = "sl_cache"; + + LLFile::mkdir(cache_dir); +} + +void LLDiskCache::purge() +{ + if (mEnableCacheDebugInfo) + { + LL_INFOS() << "Total dir size before purge is " << dirFileSize(mCacheDir) << LL_ENDL; + } + + auto start_time = std::chrono::high_resolution_clock::now(); + + typedef std::pair> file_info_t; + std::vector file_info; + +#if LL_WINDOWS + std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); +#else + std::string cache_path(mCacheDir); +#endif + if (boost::filesystem::is_directory(cache_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + uintmax_t file_size = boost::filesystem::file_size(entry); + const std::string file_path = entry.path().string(); + const std::time_t file_time = boost::filesystem::last_write_time(entry); + + file_info.push_back(file_info_t(file_time, { file_size, file_path })); + } + } + } + } + + std::sort(file_info.begin(), file_info.end(), [](file_info_t& x, file_info_t& y) + { + return x.first > y.first; + }); + + LL_INFOS() << "Purging cache to a maximum of " << mMaxSizeBytes << " bytes" << LL_ENDL; + + uintmax_t file_size_total = 0; + for (file_info_t& entry : file_info) + { + file_size_total += entry.second.first; + + std::string action = ""; + if (file_size_total > mMaxSizeBytes) + { + action = "DELETE:"; + boost::filesystem::remove(entry.second.second); + } + else + { + action = " KEEP:"; + } + + if (mEnableCacheDebugInfo) + { + // have to do this because of LL_INFO/LL_END weirdness + std::ostringstream line; + + line << action << " "; + line << entry.first << " "; + line << entry.second.first << " "; + line << entry.second.second; + line << " (" << file_size_total << "/" << mMaxSizeBytes << ")"; + LL_INFOS() << line.str() << LL_ENDL; + } + } + + if (mEnableCacheDebugInfo) + { + auto end_time = std::chrono::high_resolution_clock::now(); + auto execute_time = std::chrono::duration_cast(end_time - start_time).count(); + LL_INFOS() << "Total dir size after purge is " << dirFileSize(mCacheDir) << LL_ENDL; + LL_INFOS() << "Cache purge took " << execute_time << " ms to execute for " << file_info.size() << " files" << LL_ENDL; + } +} + +const std::string LLDiskCache::assetTypeToString(LLAssetType::EType at) +{ + /** + * Make use of the handy C++17 feature that allows + * for inline initialization of an std::map<> + */ + typedef std::map asset_type_to_name_t; + asset_type_to_name_t asset_type_to_name = + { + { LLAssetType::AT_TEXTURE, "TEXTURE" }, + { LLAssetType::AT_SOUND, "SOUND" }, + { LLAssetType::AT_CALLINGCARD, "CALLINGCARD" }, + { LLAssetType::AT_LANDMARK, "LANDMARK" }, + { LLAssetType::AT_SCRIPT, "SCRIPT" }, + { LLAssetType::AT_CLOTHING, "CLOTHING" }, + { LLAssetType::AT_OBJECT, "OBJECT" }, + { LLAssetType::AT_NOTECARD, "NOTECARD" }, + { LLAssetType::AT_CATEGORY, "CATEGORY" }, + { LLAssetType::AT_LSL_TEXT, "LSL_TEXT" }, + { LLAssetType::AT_LSL_BYTECODE, "LSL_BYTECODE" }, + { LLAssetType::AT_TEXTURE_TGA, "TEXTURE_TGA" }, + { LLAssetType::AT_BODYPART, "BODYPART" }, + { LLAssetType::AT_SOUND_WAV, "SOUND_WAV" }, + { LLAssetType::AT_IMAGE_TGA, "IMAGE_TGA" }, + { LLAssetType::AT_IMAGE_JPEG, "IMAGE_JPEG" }, + { LLAssetType::AT_ANIMATION, "ANIMATION" }, + { LLAssetType::AT_GESTURE, "GESTURE" }, + { LLAssetType::AT_SIMSTATE, "SIMSTATE" }, + { LLAssetType::AT_LINK, "LINK" }, + { LLAssetType::AT_LINK_FOLDER, "LINK_FOLDER" }, + { LLAssetType::AT_MARKETPLACE_FOLDER, "MARKETPLACE_FOLDER" }, + { LLAssetType::AT_WIDGET, "WIDGET" }, + { LLAssetType::AT_PERSON, "PERSON" }, + { LLAssetType::AT_MESH, "MESH" }, + { LLAssetType::AT_SETTINGS, "SETTINGS" }, + { LLAssetType::AT_UNKNOWN, "UNKNOWN" } + }; + + asset_type_to_name_t::iterator iter = asset_type_to_name.find(at); + if (iter != asset_type_to_name.end()) + { + return iter->second; + } + + return std::string("UNKNOWN"); +} + +const std::string LLDiskCache::metaDataToFilepath(const std::string id, + LLAssetType::EType at, + const std::string extra_info) +{ + std::ostringstream file_path; + + file_path << mCacheDir; + file_path << gDirUtilp->getDirDelimiter(); + file_path << mCacheFilenamePrefix; + file_path << "_"; + file_path << id; + file_path << "_"; + file_path << (extra_info.empty() ? "0" : extra_info); + //file_path << "_"; + //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + // for details of why it was removed. Note that if you put it + // back or change the format of the filename, the cache files + // files will be invalidated (and perhaps, more importantly, + // never deleted unless you delete them manually). + file_path << ".asset"; + + return file_path.str(); +} + +void LLDiskCache::updateFileAccessTime(const std::string file_path) +{ + /** + * Threshold in time_t units that is used to decide if the last access time + * time of the file is updated or not. Added as a precaution for the concern + * outlined in SL-14582 about frequent writes on older SSDs reducing their + * lifespan. I think this is the right place for the threshold value - rather + * than it being a pref - do comment on that Jira if you disagree... + * + * Let's start with 1 hour in time_t units and see how that unfolds + */ + const std::time_t time_threshold = 1 * 60 * 60; + + // current time + const std::time_t cur_time = std::time(nullptr); + +#if LL_WINDOWS + // file last write time + const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path)); + + // delta between cur time and last time the file was written + const std::time_t delta_time = cur_time - last_write_time; + + // we only write the new value if the time in time_threshold has elapsed + // before the last one + if (delta_time > time_threshold) + { + boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time); + } +#else + // file last write time + const std::time_t last_write_time = boost::filesystem::last_write_time(file_path); + + // delta between cur time and last time the file was written + const std::time_t delta_time = cur_time - last_write_time; + + // we only write the new value if the time in time_threshold has elapsed + // before the last one + if (delta_time > time_threshold) + { + boost::filesystem::last_write_time(file_path, cur_time); + } +#endif +} + +const std::string LLDiskCache::getCacheInfo() +{ + std::ostringstream cache_info; + + F32 max_in_mb = (F32)mMaxSizeBytes / (1024.0 * 1024.0); + F32 percent_used = ((F32)dirFileSize(mCacheDir) / (F32)mMaxSizeBytes) * 100.0; + + cache_info << std::fixed; + cache_info << std::setprecision(1); + cache_info << "Max size " << max_in_mb << " MB "; + cache_info << "(" << percent_used << "% used)"; + + return cache_info.str(); +} + +void LLDiskCache::clearCache() +{ + /** + * See notes on performance in dirFileSize(..) - there may be + * a quicker way to do this by operating on the parent dir vs + * the component files but it's called infrequently so it's + * likely just fine + */ +#if LL_WINDOWS + std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); +#else + std::string cache_path(mCacheDir); +#endif + if (boost::filesystem::is_directory(cache_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + boost::filesystem::remove(entry); + } + } + } + } +} + +uintmax_t LLDiskCache::dirFileSize(const std::string dir) +{ + uintmax_t total_file_size = 0; + + /** + * There may be a better way that works directly on the folder (similar to + * right clicking on a folder in the OS and asking for size vs right clicking + * on all files and adding up manually) but this is very fast - less than 100ms + * for 10,000 files in my testing so, so long as it's not called frequently, + * it should be okay. Note that's it's only currently used for logging/debugging + * so if performance is ever an issue, optimizing this or removing it altogether, + * is an easy win. + */ +#if LL_WINDOWS + std::wstring dir_path(utf8str_to_utf16str(dir)); +#else + std::string dir_path(dir); +#endif + if (boost::filesystem::is_directory(dir_path)) + { + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) + { + if (boost::filesystem::is_regular_file(entry)) + { + if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) + { + total_file_size += boost::filesystem::file_size(entry); + } + } + } + } + + return total_file_size; +} -- cgit v1.2.3 From d7518c7b4f5eca731d0ea143bab34b279bd4ee13 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 18:33:35 -0800 Subject: Ansariel kindly offered their patch to help mitigate this round of file system issues - taken from https://vcs.firestormviewer.org/phoenix-firestorm/changeset/104a8600946be01e2de44d10ad069ba854272d1f --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..61eb654693 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -192,8 +192,8 @@ const std::string LLDiskCache::metaDataToFilepath(const std::string id, file_path << id; file_path << "_"; file_path << (extra_info.empty() ? "0" : extra_info); - //file_path << "_"; - //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + file_path << "_"; + file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames // for details of why it was removed. Note that if you put it // back or change the format of the filename, the cache files // files will be invalidated (and perhaps, more importantly, -- cgit v1.2.3 From ad2edc1d8336c745fc897197e2e85debf3654c17 Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 9 Mar 2021 18:35:30 -0800 Subject: Remove debugging tags --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 61eb654693..c9f7684b5a 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -192,8 +192,8 @@ const std::string LLDiskCache::metaDataToFilepath(const std::string id, file_path << id; file_path << "_"; file_path << (extra_info.empty() ? "0" : extra_info); - file_path << "_"; - file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames + //file_path << "_"; + //file_path << assetTypeToString(at); // see SL-14210 Prune descriptive tag from new cache filenames // for details of why it was removed. Note that if you put it // back or change the format of the filename, the cache files // files will be invalidated (and perhaps, more importantly, -- cgit v1.2.3 From 8633ba9c8bd8e24f9b6d8a74ed7bc4d50072a728 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Tue, 11 May 2021 20:27:07 +0200 Subject: BUG-230697: Do not crash viewer during cache cleanup --- indra/llfilesystem/lldiskcache.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..75255ef230 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -102,7 +102,12 @@ void LLDiskCache::purge() if (file_size_total > mMaxSizeBytes) { action = "DELETE:"; - boost::filesystem::remove(entry.second.second); + boost::system::error_code ec; + boost::filesystem::remove(entry.second.second, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to delete cache file " << entry.second.second << ": " << ec.message() << LL_ENDL; + } } else { @@ -284,7 +289,12 @@ void LLDiskCache::clearCache() { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - boost::filesystem::remove(entry); + boost::system::error_code ec; + boost::filesystem::remove(entry, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to delete cache file " << entry << ": " << ec.message() << LL_ENDL; + } } } } -- cgit v1.2.3 From 3898609ae24e7787d6f6ae71c2424b43102326bf Mon Sep 17 00:00:00 2001 From: Callum Prentice Date: Tue, 11 May 2021 12:58:05 -0700 Subject: Fix for SL-15226 Simple cache viewer: Integer overflow in cache size - via FS:Ansariel --- indra/llfilesystem/lldiskcache.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c9f7684b5a..241c46dc73 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -40,7 +40,7 @@ #include "lldiskcache.h" LLDiskCache::LLDiskCache(const std::string cache_dir, - const int max_size_bytes, + const uintmax_t max_size_bytes, const bool enable_cache_debug_info) : mCacheDir(cache_dir), mMaxSizeBytes(max_size_bytes), -- cgit v1.2.3 From 0e253cb9098cb58b2f3528860a5fd9f00ec5af96 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Wed, 12 May 2021 10:45:23 +0200 Subject: BUG-230673: Trim asset disk cache regularly --- indra/llfilesystem/lldiskcache.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 02678864b9..68423cc4f8 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -335,3 +335,25 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) return total_file_size; } + +LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : + LLThread("PurgeDiskCacheThread", nullptr) +{ +} + +void LLPurgeDiskCacheThread::run() +{ + constexpr F64 CHECK_INTERVAL = 60; + mTimer.setTimerExpirySec(CHECK_INTERVAL); + mTimer.start(); + + do + { + if (mTimer.checkExpirationAndReset(CHECK_INTERVAL)) + { + LLDiskCache::instance().purge(); + } + + ms_sleep(100); + } while (!isQuitting()); +} \ No newline at end of file -- cgit v1.2.3 From 89cf988aaf0de402641cd945e7e9ad5292bc78d6 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Mon, 17 May 2021 09:49:32 +0200 Subject: BUG-230673: Add warning that LLDiskCache::purge() is also called from outside the main thread --- indra/llfilesystem/lldiskcache.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 68423cc4f8..17906ce369 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -51,6 +51,8 @@ LLDiskCache::LLDiskCache(const std::string cache_dir, LLFile::mkdir(cache_dir); } +// WARNING: purge() is called by LLPurgeDiskCacheThread. As such it must +// NOT touch any LLDiskCache data without introducing and locking a mutex! void LLDiskCache::purge() { if (mEnableCacheDebugInfo) -- cgit v1.2.3 From 87faf258911f5d23416500ff632050ce05b30e3e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 May 2021 10:24:27 -0400 Subject: SL-15200: Explain why purge() is called on another thread. Also add Ansariel's explanation for why interaction through the filesystem itself should be safe. --- indra/llfilesystem/lldiskcache.cpp | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 17906ce369..c0f10ac620 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -53,6 +53,32 @@ 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) @@ -358,4 +384,4 @@ void LLPurgeDiskCacheThread::run() ms_sleep(100); } while (!isQuitting()); -} \ No newline at end of file +} -- cgit v1.2.3 From b3708ac238d51eaf808cb77a4493e518c1593e33 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 17 May 2021 15:10:06 -0400 Subject: SL-15200: Use new LLApp::sleep() in LLPurgeDiskCacheThread::run(). --- indra/llfilesystem/lldiskcache.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index c0f10ac620..6b0bbaeb48 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 @@ -371,17 +372,10 @@ LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : void LLPurgeDiskCacheThread::run() { - constexpr F64 CHECK_INTERVAL = 60; - mTimer.setTimerExpirySec(CHECK_INTERVAL); - mTimer.start(); + constexpr long CHECK_INTERVAL = 60; - do + while (LLApp::instance()->sleep(std::chrono::seconds(CHECK_INTERVAL))) { - if (mTimer.checkExpirationAndReset(CHECK_INTERVAL)) - { - LLDiskCache::instance().purge(); - } - - ms_sleep(100); - } while (!isQuitting()); + LLDiskCache::instance().purge(); + } } -- cgit v1.2.3 From ac8640d338997020ca0650001ff004e1103ac5cb Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 18 May 2021 09:51:45 -0400 Subject: SL-15200: LLPurgeDiskCacheThread's CHECK_INTERVAL is secs. --- indra/llfilesystem/lldiskcache.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 6b0bbaeb48..905351886f 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -372,9 +372,9 @@ LLPurgeDiskCacheThread::LLPurgeDiskCacheThread() : void LLPurgeDiskCacheThread::run() { - constexpr long CHECK_INTERVAL = 60; + constexpr std::chrono::seconds CHECK_INTERVAL{60}; - while (LLApp::instance()->sleep(std::chrono::seconds(CHECK_INTERVAL))) + while (LLApp::instance()->sleep(CHECK_INTERVAL)) { LLDiskCache::instance().purge(); } -- cgit v1.2.3 From 4c558e85bd90d77696e9201b8996272176d7adc5 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Thu, 10 Jun 2021 01:09:31 +0200 Subject: Fix more crashes in disk cache due to boost error handling --- indra/llfilesystem/lldiskcache.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index 905351886f..f4d20d0e61 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -253,9 +253,15 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // current time const std::time_t cur_time = std::time(nullptr); + boost::system::error_code ec; #if LL_WINDOWS // file last write time - const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path)); + const std::time_t last_write_time = boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + return; + } // delta between cur time and last time the file was written const std::time_t delta_time = cur_time - last_write_time; @@ -264,11 +270,16 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // before the last one if (delta_time > time_threshold) { - boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time); + boost::filesystem::last_write_time(utf8str_to_utf16str(file_path), cur_time, ec); } #else // file last write time - const std::time_t last_write_time = boost::filesystem::last_write_time(file_path); + const std::time_t last_write_time = boost::filesystem::last_write_time(file_path, ec); + if (ec.failed()) + { + LL_WARNS() << "Failed to read last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + return; + } // delta between cur time and last time the file was written const std::time_t delta_time = cur_time - last_write_time; @@ -277,9 +288,14 @@ void LLDiskCache::updateFileAccessTime(const std::string file_path) // before the last one if (delta_time > time_threshold) { - boost::filesystem::last_write_time(file_path, cur_time); + boost::filesystem::last_write_time(file_path, cur_time, ec); } #endif + + if (ec.failed()) + { + LL_WARNS() << "Failed to update last write time for cache file " << file_path << ": " << ec.message() << LL_ENDL; + } } const std::string LLDiskCache::getCacheInfo() -- cgit v1.2.3 From 564a7acb321c54ad5f9fc6f5242efbe4c7638dbc Mon Sep 17 00:00:00 2001 From: Ansariel Date: Fri, 11 Jun 2021 22:50:39 +0200 Subject: Change all remaining boost::filesystem methods to their non-throwing overloads --- indra/llfilesystem/lldiskcache.cpp | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index f4d20d0e61..dd8916dccb 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -87,6 +87,7 @@ void LLDiskCache::purge() LL_INFOS() << "Total dir size before purge is " << dirFileSize(mCacheDir) << LL_ENDL; } + boost::system::error_code ec; auto start_time = std::chrono::high_resolution_clock::now(); typedef std::pair> file_info_t; @@ -97,17 +98,25 @@ void LLDiskCache::purge() #else std::string cache_path(mCacheDir); #endif - if (boost::filesystem::is_directory(cache_path)) + if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - uintmax_t file_size = boost::filesystem::file_size(entry); + uintmax_t file_size = boost::filesystem::file_size(entry, ec); + if (ec.failed()) + { + continue; + } const std::string file_path = entry.path().string(); - const std::time_t file_time = boost::filesystem::last_write_time(entry); + const std::time_t file_time = boost::filesystem::last_write_time(entry, ec); + if (ec.failed()) + { + continue; + } file_info.push_back(file_info_t(file_time, { file_size, file_path })); } @@ -131,7 +140,6 @@ void LLDiskCache::purge() if (file_size_total > mMaxSizeBytes) { action = "DELETE:"; - boost::system::error_code ec; boost::filesystem::remove(entry.second.second, ec); if (ec.failed()) { @@ -321,20 +329,20 @@ void LLDiskCache::clearCache() * the component files but it's called infrequently so it's * likely just fine */ + boost::system::error_code ec; #if LL_WINDOWS std::wstring cache_path(utf8str_to_utf16str(mCacheDir)); #else std::string cache_path(mCacheDir); #endif - if (boost::filesystem::is_directory(cache_path)) + if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - boost::system::error_code ec; boost::filesystem::remove(entry, ec); if (ec.failed()) { @@ -359,20 +367,25 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) * so if performance is ever an issue, optimizing this or removing it altogether, * is an easy win. */ + boost::system::error_code ec; #if LL_WINDOWS std::wstring dir_path(utf8str_to_utf16str(dir)); #else std::string dir_path(dir); #endif - if (boost::filesystem::is_directory(dir_path)) + if (boost::filesystem::is_directory(dir_path, ec) && !ec.failed()) { for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) { - if (boost::filesystem::is_regular_file(entry)) + if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { if (entry.path().string().find(mCacheFilenamePrefix) != std::string::npos) { - total_file_size += boost::filesystem::file_size(entry); + uintmax_t file_size = boost::filesystem::file_size(entry, ec); + if (!ec.failed()) + { + total_file_size += file_size; + } } } } -- cgit v1.2.3 From fedae88be7b7174956f549194b13883ff2cb4161 Mon Sep 17 00:00:00 2001 From: Ansariel Date: Fri, 11 Jun 2021 22:59:55 +0200 Subject: boost::filesystem::directory_iterator uses throw-behavior by default as well --- indra/llfilesystem/lldiskcache.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llfilesystem/lldiskcache.cpp') diff --git a/indra/llfilesystem/lldiskcache.cpp b/indra/llfilesystem/lldiskcache.cpp index dd8916dccb..ee43a599f7 100644 --- a/indra/llfilesystem/lldiskcache.cpp +++ b/indra/llfilesystem/lldiskcache.cpp @@ -100,7 +100,7 @@ void LLDiskCache::purge() #endif if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { @@ -337,7 +337,7 @@ void LLDiskCache::clearCache() #endif if (boost::filesystem::is_directory(cache_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(cache_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { @@ -375,7 +375,7 @@ uintmax_t LLDiskCache::dirFileSize(const std::string dir) #endif if (boost::filesystem::is_directory(dir_path, ec) && !ec.failed()) { - for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path), {})) + for (auto& entry : boost::make_iterator_range(boost::filesystem::directory_iterator(dir_path, ec), {})) { if (boost::filesystem::is_regular_file(entry, ec) && !ec.failed()) { -- cgit v1.2.3