fix: change ASAR archive cache to per-process to fix leak (#29292)

shell/browser/electron_browser_main_parts.cc

@@ -207,9 +207,7 @@ ElectronBrowserMainParts::ElectronBrowserMainParts(
   self_ = this;
 }
 
-ElectronBrowserMainParts::~ElectronBrowserMainParts() {
-  asar::ClearArchives();
-}
+ElectronBrowserMainParts::~ElectronBrowserMainParts() = default;
 
 // static
 ElectronBrowserMainParts* ElectronBrowserMainParts::Get() {

shell/common/asar/archive.cc

@@ -141,6 +141,11 @@ Archive::~Archive() {
 }
 
 bool Archive::Init() {
+  if (header_)
+    return true;
+
+  base::AutoLock auto_lock(lock_);
+
   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<base::FilePath>* list) {
+                      std::vector<base::FilePath>* list) 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(lock_);
+
   auto it = external_files_.find(path.value());
   if (it != external_files_.end()) {
     *out = it->second->path();

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.
 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<base::FilePath>* files);
+  bool Readdir(const base::FilePath& path,
+               std::vector<base::FilePath>* 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,9 +67,9 @@ class Archive {
   int GetFD() const;
 
   base::FilePath path() const { return path_; }
-  base::DictionaryValue* header() const { return header_.get(); }
 
  private:
+  base::Lock lock_;
   base::FilePath path_;
   base::File file_;
   int fd_ = -1;

shell/common/asar/asar_util.cc

@@ -6,11 +6,14 @@
 
 #include <map>
 #include <string>
+#include <utility>
 
 #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,39 @@ namespace asar {
 
 namespace {
 
-// The global instance of ArchiveMap, will be destroyed on exit.
 typedef std::map<base::FilePath, std::shared_ptr<Archive>> ArchiveMap;
-base::LazyInstance<base::ThreadLocalPointer<ArchiveMap>>::Leaky
-    g_archive_map_tls = LAZY_INSTANCE_INITIALIZER;
 
 const base::FilePath::CharType kAsarExtension[] = FILE_PATH_LITERAL(".asar");
 
-std::map<base::FilePath, bool> 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<std::map<base::FilePath, bool>>
+      s_is_directory_cache;
+  static base::NoDestructor<base::Lock> 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
 
+std::pair<ArchiveMap&, base::Lock&> GetArchiveCache() {
+  static base::NoDestructor<ArchiveMap> s_archive_map;
+  static base::NoDestructor<base::Lock> lock;
+
+  return std::pair<ArchiveMap&, base::Lock&>(*s_archive_map, *lock);
+}
+
 std::shared_ptr<Archive> 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();
+  std::pair<ArchiveMap&, base::Lock&> archiveMapAndLock = GetArchiveCache();
+  ArchiveMap& map = archiveMapAndLock.first;
+  base::AutoLock auto_lock(archiveMapAndLock.second);
 
   // if we have it, return it
   const auto lower = map.lower_bound(path);
@@ -61,8 +73,11 @@ std::shared_ptr<Archive> GetOrCreateAsarArchive(const base::FilePath& path) {
 }
 
 void ClearArchives() {
-  if (g_archive_map_tls.Pointer()->Get())
-    delete g_archive_map_tls.Pointer()->Get();
+  std::pair<ArchiveMap&, base::Lock&> archiveMapAndLock = GetArchiveCache();
+  ArchiveMap& map = archiveMapAndLock.first;
+  base::AutoLock auto_lock(archiveMapAndLock.second);
+
+  map.clear();
 }
 
 bool GetAsarArchivePath(const base::FilePath& full_path,

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<Archive> GetOrCreateAsarArchive(const base::FilePath& path);
 
 // Destroy cached Archive objects.

shell/renderer/electron_renderer_client.cc

@@ -39,9 +39,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) {

shell/renderer/web_worker_observer.cc

@@ -39,7 +39,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(