Revert "fix: change ASAR archive cache to per-process to fix leak (#29535)"

This reverts commit ff2a0a4bd3.
This commit is contained in:
Samuel Attard
2021-06-04 10:28:20 -07:00
parent 0753e4911c
commit 0dcc660794
7 changed files with 36 additions and 58 deletions

View File

@@ -44,6 +44,7 @@
#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"
@@ -210,7 +211,9 @@ ElectronBrowserMainParts::ElectronBrowserMainParts(
self_ = this;
}
ElectronBrowserMainParts::~ElectronBrowserMainParts() = default;
ElectronBrowserMainParts::~ElectronBrowserMainParts() {
asar::ClearArchives();
}
// static
ElectronBrowserMainParts* ElectronBrowserMainParts::Get() {

View File

@@ -8,7 +8,6 @@
#include <utility>
#include <vector>
#include "base/check.h"
#include "base/files/file.h"
#include "base/files/file_util.h"
#include "base/json/json_reader.h"
@@ -119,7 +118,7 @@ bool FillFileInfoWithNode(Archive::FileInfo* info,
} // namespace
Archive::Archive(const base::FilePath& path)
: initialized_(false), path_(path), file_(base::File::FILE_OK) {
: 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)
@@ -142,10 +141,6 @@ 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() << ": "
@@ -203,7 +198,7 @@ bool Archive::Init() {
return true;
}
bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) const {
bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) {
if (!header_)
return false;
@@ -218,7 +213,7 @@ bool Archive::GetFileInfo(const base::FilePath& path, FileInfo* info) const {
return FillFileInfoWithNode(info, header_size_, node);
}
bool Archive::Stat(const base::FilePath& path, Stats* stats) const {
bool Archive::Stat(const base::FilePath& path, Stats* stats) {
if (!header_)
return false;
@@ -242,7 +237,7 @@ bool Archive::Stat(const base::FilePath& path, Stats* stats) const {
}
bool Archive::Readdir(const base::FilePath& path,
std::vector<base::FilePath>* files) const {
std::vector<base::FilePath>* files) {
if (!header_)
return false;
@@ -262,8 +257,7 @@ bool Archive::Readdir(const base::FilePath& path,
return true;
}
bool Archive::Realpath(const base::FilePath& path,
base::FilePath* realpath) const {
bool Archive::Realpath(const base::FilePath& path, base::FilePath* realpath) {
if (!header_)
return false;
@@ -282,11 +276,6 @@ bool Archive::Realpath(const base::FilePath& path,
}
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();

View File

@@ -11,7 +11,6 @@
#include "base/files/file.h"
#include "base/files/file_path.h"
#include "base/synchronization/lock.h"
namespace base {
class DictionaryValue;
@@ -22,7 +21,7 @@ namespace asar {
class ScopedTemporaryFile;
// This class represents an asar package, and provides methods to read
// information from it. It is thread-safe after |Init| has been called.
// information from it.
class Archive {
public:
struct FileInfo {
@@ -47,17 +46,16 @@ class Archive {
bool Init();
// Get the info of a file.
bool GetFileInfo(const base::FilePath& path, FileInfo* info) const;
bool GetFileInfo(const base::FilePath& path, FileInfo* info);
// Fs.stat(path).
bool Stat(const base::FilePath& path, Stats* stats) const;
bool Stat(const base::FilePath& path, Stats* stats);
// Fs.readdir(path).
bool Readdir(const base::FilePath& path,
std::vector<base::FilePath>* files) const;
bool Readdir(const base::FilePath& path, std::vector<base::FilePath>* files);
// Fs.realpath(path).
bool Realpath(const base::FilePath& path, base::FilePath* realpath) const;
bool Realpath(const base::FilePath& path, base::FilePath* realpath);
// Copy the file into a temporary file, and return the new path.
// For unpacked file, this method will return its real path.
@@ -67,17 +65,16 @@ class Archive {
int GetFD() const;
base::FilePath path() const { return path_; }
base::DictionaryValue* header() const { return header_.get(); }
private:
bool initialized_;
const base::FilePath path_;
base::FilePath path_;
base::File file_;
int fd_ = -1;
uint32_t header_size_ = 0;
std::unique_ptr<base::DictionaryValue> header_;
// Cached external temporary files.
base::Lock external_files_lock_;
std::unordered_map<base::FilePath::StringType,
std::unique_ptr<ScopedTemporaryFile>>
external_files_;

View File

@@ -6,14 +6,11 @@
#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"
@@ -22,41 +19,30 @@ 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) {
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()) {
auto it = g_is_directory_cache.find(path);
if (it != g_is_directory_cache.end()) {
return it->second;
}
base::ThreadRestrictions::ScopedAllowIO allow_io;
return is_directory_cache[path] = base::DirectoryExists(path);
return g_is_directory_cache[path] = base::DirectoryExists(path);
}
} // namespace
ArchiveMap& GetArchiveCache() {
static base::NoDestructor<ArchiveMap> s_archive_map;
return *s_archive_map;
}
base::Lock& GetArchiveCacheLock() {
static base::NoDestructor<base::Lock> lock;
return *lock;
}
std::shared_ptr<Archive> GetOrCreateAsarArchive(const base::FilePath& path) {
base::AutoLock auto_lock(GetArchiveCacheLock());
ArchiveMap& map = GetArchiveCache();
if (!g_archive_map_tls.Pointer()->Get())
g_archive_map_tls.Pointer()->Set(new ArchiveMap);
ArchiveMap& map = *g_archive_map_tls.Pointer()->Get();
// if we have it, return it
const auto lower = map.lower_bound(path);
@@ -75,10 +61,8 @@ std::shared_ptr<Archive> GetOrCreateAsarArchive(const base::FilePath& path) {
}
void ClearArchives() {
base::AutoLock auto_lock(GetArchiveCacheLock());
ArchiveMap& map = GetArchiveCache();
map.clear();
if (g_archive_map_tls.Pointer()->Get())
delete g_archive_map_tls.Pointer()->Get();
}
bool GetAsarArchivePath(const base::FilePath& full_path,

View File

@@ -16,7 +16,7 @@ namespace asar {
class Archive;
// Gets or creates and caches a new Archive from the path.
// Gets or creates a new Archive from the path.
std::shared_ptr<Archive> GetOrCreateAsarArchive(const base::FilePath& path);
// Destroy cached Archive objects.

View File

@@ -11,6 +11,7 @@
#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"
@@ -39,7 +40,9 @@ ElectronRendererClient::ElectronRendererClient()
NodeBindings::Create(NodeBindings::BrowserEnvironment::kRenderer)),
electron_bindings_(new ElectronBindings(node_bindings_->uv_loop())) {}
ElectronRendererClient::~ElectronRendererClient() = default;
ElectronRendererClient::~ElectronRendererClient() {
asar::ClearArchives();
}
void ElectronRendererClient::RenderFrameCreated(
content::RenderFrame* render_frame) {

View File

@@ -7,6 +7,7 @@
#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"
@@ -38,6 +39,7 @@ WebWorkerObserver::~WebWorkerObserver() {
lazy_tls.Pointer()->Set(nullptr);
node::FreeEnvironment(node_bindings_->uv_env());
node::FreeIsolateData(node_bindings_->isolate_data());
asar::ClearArchives();
}
void WebWorkerObserver::WorkerScriptReadyForEvaluation(