diff --git a/shell/browser/electron_browser_main_parts.cc b/shell/browser/electron_browser_main_parts.cc index 3d2aabedcbf8..0b0dcf8a0ee6 100644 --- a/shell/browser/electron_browser_main_parts.cc +++ b/shell/browser/electron_browser_main_parts.cc @@ -43,7 +43,6 @@ #include "shell/browser/ui/devtools_manager_delegate.h" #include "shell/common/api/electron_bindings.h" #include "shell/common/application_info.h" -#include "shell/common/asar/asar_util.h" #include "shell/common/electron_paths.h" #include "shell/common/gin_helper/trackable_object.h" #include "shell/common/node_bindings.h" @@ -209,9 +208,7 @@ ElectronBrowserMainParts::ElectronBrowserMainParts( self_ = this; } -ElectronBrowserMainParts::~ElectronBrowserMainParts() { - asar::ClearArchives(); -} +ElectronBrowserMainParts::~ElectronBrowserMainParts() = default; // static ElectronBrowserMainParts* ElectronBrowserMainParts::Get() { diff --git a/shell/common/asar/archive.cc b/shell/common/asar/archive.cc index 1b429c992964..e783c90098a7 100644 --- a/shell/common/asar/archive.cc +++ b/shell/common/asar/archive.cc @@ -8,6 +8,7 @@ #include #include +#include "base/check.h" #include "base/files/file.h" #include "base/files/file_util.h" #include "base/json/json_reader.h" @@ -118,7 +119,7 @@ bool FillFileInfoWithNode(Archive::FileInfo* info, } // namespace Archive::Archive(const base::FilePath& path) - : path_(path), file_(base::File::FILE_OK) { + : initialized_(false), path_(path), file_(base::File::FILE_OK) { base::ThreadRestrictions::ScopedAllowIO allow_io; file_.Initialize(path_, base::File::FLAG_OPEN | base::File::FLAG_READ); #if defined(OS_WIN) @@ -141,6 +142,10 @@ Archive::~Archive() { } bool Archive::Init() { + // Should only be initialized once + CHECK(!initialized_); + initialized_ = true; + if (!file_.IsValid()) { if (file_.error_details() != base::File::FILE_ERROR_NOT_FOUND) { LOG(WARNING) << "Opening " << path_.value() << ": " @@ -198,7 +203,7 @@ bool Archive::Init() { return true; } -bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) { +bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) const { if (!header_) return false; @@ -213,7 +218,7 @@ bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) { return FillFileInfoWithNode(info, header_size_, node); } -bool Archive::Stat(const base::FilePath& path, Stats* stats) { +bool Archive::Stat(const base::FilePath& path, Stats* stats) const { if (!header_) return false; @@ -237,7 +242,7 @@ bool Archive::Stat(const base::FilePath& path, Stats* stats) { } bool Archive::Readdir(const base::FilePath& path, - std::vector* files) { + std::vector* files) const { if (!header_) return false; @@ -257,7 +262,8 @@ bool Archive::Readdir(const base::FilePath& path, return true; } -bool Archive::Realpath(const base::FilePath& path, base::FilePath* realpath) { +bool Archive::Realpath(const base::FilePath& path, + base::FilePath* realpath) const { if (!header_) return false; @@ -276,6 +282,11 @@ bool Archive::Realpath(const base::FilePath& path, base::FilePath* realpath) { } bool Archive::CopyFileOut(const base::FilePath& path, base::FilePath* out) { + if (!header_) + return false; + + base::AutoLock auto_lock(external_files_lock_); + auto it = external_files_.find(path.value()); if (it != external_files_.end()) { *out = it->second->path(); diff --git a/shell/common/asar/archive.h b/shell/common/asar/archive.h index d97cb03cc781..a25ae4d4de30 100644 --- a/shell/common/asar/archive.h +++ b/shell/common/asar/archive.h @@ -11,6 +11,7 @@ #include "base/files/file.h" #include "base/files/file_path.h" +#include "base/synchronization/lock.h" namespace base { class DictionaryValue; @@ -21,7 +22,7 @@ namespace asar { class ScopedTemporaryFile; // This class represents an asar package, and provides methods to read -// information from it. +// information from it. It is thread-safe after |Init| has been called. class Archive { public: struct FileInfo { @@ -46,16 +47,17 @@ class Archive { bool Init(); // Get the info of a file. - bool GetFileInfo(const base::FilePath& path, FileInfo* info); + bool GetFileInfo(const base::FilePath& path, FileInfo* info) const; // Fs.stat(path). - bool Stat(const base::FilePath& path, Stats* stats); + bool Stat(const base::FilePath& path, Stats* stats) const; // Fs.readdir(path). - bool Readdir(const base::FilePath& path, std::vector* files); + bool Readdir(const base::FilePath& path, + std::vector* files) const; // Fs.realpath(path). - bool Realpath(const base::FilePath& path, base::FilePath* realpath); + bool Realpath(const base::FilePath& path, base::FilePath* realpath) const; // Copy the file into a temporary file, and return the new path. // For unpacked file, this method will return its real path. @@ -65,16 +67,17 @@ class Archive { int GetFD() const; base::FilePath path() const { return path_; } - base::DictionaryValue* header() const { return header_.get(); } private: - base::FilePath path_; + bool initialized_; + const base::FilePath path_; base::File file_; int fd_ = -1; uint32_t header_size_ = 0; std::unique_ptr header_; // Cached external temporary files. + base::Lock external_files_lock_; std::unordered_map> external_files_; diff --git a/shell/common/asar/asar_util.cc b/shell/common/asar/asar_util.cc index 65ddbf2843dc..4800fb13421a 100644 --- a/shell/common/asar/asar_util.cc +++ b/shell/common/asar/asar_util.cc @@ -6,11 +6,14 @@ #include #include +#include #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/lazy_instance.h" +#include "base/no_destructor.h" #include "base/stl_util.h" +#include "base/synchronization/lock.h" #include "base/threading/thread_local.h" #include "base/threading/thread_restrictions.h" #include "shell/common/asar/archive.h" @@ -19,30 +22,41 @@ namespace asar { namespace { -// The global instance of ArchiveMap, will be destroyed on exit. typedef std::map> ArchiveMap; -base::LazyInstance>::Leaky - g_archive_map_tls = LAZY_INSTANCE_INITIALIZER; const base::FilePath::CharType kAsarExtension[] = FILE_PATH_LITERAL(".asar"); -std::map g_is_directory_cache; - bool IsDirectoryCached(const base::FilePath& path) { - auto it = g_is_directory_cache.find(path); - if (it != g_is_directory_cache.end()) { + static base::NoDestructor> + s_is_directory_cache; + static base::NoDestructor lock; + + base::AutoLock auto_lock(*lock); + auto& is_directory_cache = *s_is_directory_cache; + + auto it = is_directory_cache.find(path); + if (it != is_directory_cache.end()) { return it->second; } base::ThreadRestrictions::ScopedAllowIO allow_io; - return g_is_directory_cache[path] = base::DirectoryExists(path); + return is_directory_cache[path] = base::DirectoryExists(path); } } // namespace +ArchiveMap& GetArchiveCache() { + static base::NoDestructor s_archive_map; + return *s_archive_map; +} + +base::Lock& GetArchiveCacheLock() { + static base::NoDestructor lock; + return *lock; +} + std::shared_ptr GetOrCreateAsarArchive(const base::FilePath& path) { - if (!g_archive_map_tls.Pointer()->Get()) - g_archive_map_tls.Pointer()->Set(new ArchiveMap); - ArchiveMap& map = *g_archive_map_tls.Pointer()->Get(); + base::AutoLock auto_lock(GetArchiveCacheLock()); + ArchiveMap& map = GetArchiveCache(); // if we have it, return it const auto lower = map.lower_bound(path); @@ -61,8 +75,10 @@ std::shared_ptr GetOrCreateAsarArchive(const base::FilePath& path) { } void ClearArchives() { - if (g_archive_map_tls.Pointer()->Get()) - delete g_archive_map_tls.Pointer()->Get(); + base::AutoLock auto_lock(GetArchiveCacheLock()); + ArchiveMap& map = GetArchiveCache(); + + map.clear(); } bool GetAsarArchivePath(const base::FilePath& full_path, diff --git a/shell/common/asar/asar_util.h b/shell/common/asar/asar_util.h index c48336eb2c61..efeaf59a16bb 100644 --- a/shell/common/asar/asar_util.h +++ b/shell/common/asar/asar_util.h @@ -16,7 +16,7 @@ namespace asar { class Archive; -// Gets or creates a new Archive from the path. +// Gets or creates and caches a new Archive from the path. std::shared_ptr GetOrCreateAsarArchive(const base::FilePath& path); // Destroy cached Archive objects. diff --git a/shell/renderer/electron_renderer_client.cc b/shell/renderer/electron_renderer_client.cc index d55e0a067bfe..ff2f2ec0c6e7 100644 --- a/shell/renderer/electron_renderer_client.cc +++ b/shell/renderer/electron_renderer_client.cc @@ -10,7 +10,6 @@ #include "content/public/renderer/render_frame.h" #include "electron/buildflags/buildflags.h" #include "shell/common/api/electron_bindings.h" -#include "shell/common/asar/asar_util.h" #include "shell/common/gin_helper/dictionary.h" #include "shell/common/gin_helper/event_emitter_caller.h" #include "shell/common/node_bindings.h" @@ -38,9 +37,7 @@ ElectronRendererClient::ElectronRendererClient() NodeBindings::Create(NodeBindings::BrowserEnvironment::kRenderer)), electron_bindings_(new ElectronBindings(node_bindings_->uv_loop())) {} -ElectronRendererClient::~ElectronRendererClient() { - asar::ClearArchives(); -} +ElectronRendererClient::~ElectronRendererClient() = default; void ElectronRendererClient::RenderFrameCreated( content::RenderFrame* render_frame) { diff --git a/shell/renderer/web_worker_observer.cc b/shell/renderer/web_worker_observer.cc index 7f919f983979..7bd69ebc3114 100644 --- a/shell/renderer/web_worker_observer.cc +++ b/shell/renderer/web_worker_observer.cc @@ -7,7 +7,6 @@ #include "base/lazy_instance.h" #include "base/threading/thread_local.h" #include "shell/common/api/electron_bindings.h" -#include "shell/common/asar/asar_util.h" #include "shell/common/gin_helper/event_emitter_caller.h" #include "shell/common/node_bindings.h" #include "shell/common/node_includes.h" @@ -39,7 +38,6 @@ WebWorkerObserver::~WebWorkerObserver() { lazy_tls.Pointer()->Set(nullptr); node::FreeEnvironment(node_bindings_->uv_env()); node::FreeIsolateData(node_bindings_->isolate_data()); - asar::ClearArchives(); } void WebWorkerObserver::WorkerScriptReadyForEvaluation(