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

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

* chore: address code review comments

* chore: tighten up thread-safety

* chore: better address code review comments

* chore: more code review changes
This commit is contained in:
David Sanders 2021-06-03 18:49:08 -07:00 коммит произвёл GitHub
Родитель 00693bab30
Коммит b1d1ac6524
7 изменённых файлов: 58 добавлений и 36 удалений

Просмотреть файл

@ -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() {

Просмотреть файл

@ -8,6 +8,7 @@
#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"
@ -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<base::FilePath>* files) {
std::vector<base::FilePath>* 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();

Просмотреть файл

@ -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<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,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<base::DictionaryValue> header_;
// Cached external temporary files.
base::Lock external_files_lock_;
std::unordered_map<base::FilePath::StringType,
std::unique_ptr<ScopedTemporaryFile>>
external_files_;

Просмотреть файл

@ -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,41 @@ 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
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) {
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<Archive> 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,

Просмотреть файл

@ -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.

Просмотреть файл

@ -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) {

Просмотреть файл

@ -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(