Skip to content

Commit

Permalink
Transfer v8 snapshot files as file descriptors to child processes on …
Browse files Browse the repository at this point in the history
…Posix.

An update on Chrome could replace the V8 snapshot files with newer version.
For zygoted processes this is OK because the zygote will have already mapped
the V8 snapshot and thus child processes will use the correct version of the
snapshot.  However, for processes which don't use the zygote (such as
unsandboxed plugin processes) base::LaunchProcess will launch the old
version of he Chrome binary (via /proc/self/exe on Linux), but the child will
read the new version of the V8 snapshot, thus causing a crash due to a
version mismatch.

The fix is to load V8 snapshot file in the browser and pass a file descriptor to
the child processes (much like Android already did, but for different reasons).
This ensures that the child process always sees the correct version of the
snapshot file.

BUG=457656,461057

Review URL: https://codereview.chromium.org/944913002

Cr-Commit-Position: refs/heads/master@{#317790}
  • Loading branch information
rmcilroy authored and Commit bot committed Feb 24, 2015
1 parent 4d52e51 commit 3fb0727
Show file tree
Hide file tree
Showing 10 changed files with 115 additions and 67 deletions.
2 changes: 1 addition & 1 deletion build/android/pylib/utils/isolator.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def DefaultConfigVariables():
'use_instrumented_libraries': '0',
'use_openssl': '0',
'use_ozone': '0',
'v8_use_external_startup_data': '0',
'v8_use_external_startup_data': '1',
}


Expand Down
47 changes: 26 additions & 21 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,10 @@ namespace chrome {

ChromeContentBrowserClient::ChromeContentBrowserClient()
: prerender_tracker_(NULL),
#if defined(OS_POSIX) && !defined(OS_MACOSX)
v8_natives_fd_(-1),
v8_snapshot_fd_(-1),
#endif // OS_POSIX && !OS_MACOSX
weak_factory_(this) {
#if defined(ENABLE_PLUGINS)
for (size_t i = 0; i < arraysize(kPredefinedAllowedDevChannelOrigins); ++i)
Expand Down Expand Up @@ -2401,6 +2405,28 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
const base::CommandLine& command_line,
int child_process_id,
FileDescriptorInfo* mappings) {
#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
if (v8_snapshot_fd_.get() == -1 && v8_natives_fd_.get() == -1) {
base::FilePath v8_data_path;
PathService::Get(gin::IsolateHolder::kV8SnapshotBasePathKey, &v8_data_path);
DCHECK(!v8_data_path.empty());

int file_flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
base::FilePath v8_natives_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kNativesFileName);
base::FilePath v8_snapshot_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kSnapshotFileName);
base::File v8_natives_data_file(v8_natives_data_path, file_flags);
base::File v8_snapshot_data_file(v8_snapshot_data_path, file_flags);
DCHECK(v8_natives_data_file.IsValid());
DCHECK(v8_snapshot_data_file.IsValid());
v8_natives_fd_.reset(v8_natives_data_file.TakePlatformFile());
v8_snapshot_fd_.reset(v8_snapshot_data_file.TakePlatformFile());
}
mappings->Share(kV8NativesDataDescriptor, v8_natives_fd_.get());
mappings->Share(kV8SnapshotDataDescriptor, v8_snapshot_fd_.get());
#endif // V8_USE_EXTERNAL_STARTUP_DATA

#if defined(OS_ANDROID)
base::FilePath data_path;
PathService::Get(ui::DIR_RESOURCE_PAKS_ANDROID, &data_path);
Expand Down Expand Up @@ -2452,27 +2478,6 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
DCHECK(icudata_file.IsValid());
mappings->Transfer(kAndroidICUDataDescriptor,
base::ScopedFD(icudata_file.TakePlatformFile()));

#ifdef V8_USE_EXTERNAL_STARTUP_DATA
base::FilePath v8_data_path;
PathService::Get(base::DIR_ANDROID_APP_DATA, &v8_data_path);
DCHECK(!v8_data_path.empty());

int file_flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
base::FilePath v8_natives_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kNativesFileName);
base::FilePath v8_snapshot_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kSnapshotFileName);
base::File v8_natives_data_file(v8_natives_data_path, file_flags);
base::File v8_snapshot_data_file(v8_snapshot_data_path, file_flags);
DCHECK(v8_natives_data_file.IsValid());
DCHECK(v8_snapshot_data_file.IsValid());
mappings->Transfer(kV8NativesDataDescriptor,
base::ScopedFD(v8_natives_data_file.TakePlatformFile()));
mappings->Transfer(kV8SnapshotDataDescriptor,
base::ScopedFD(v8_snapshot_data_file.TakePlatformFile()));
#endif // V8_USE_EXTERNAL_STARTUP_DATA

#else
int crash_signal_fd = GetCrashSignalFD(command_line);
if (crash_signal_fd >= 0) {
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/chrome_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,11 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
// created. It is used only the IO thread.
prerender::PrerenderTracker* prerender_tracker_;

#if defined(OS_POSIX) && !defined(OS_MACOSX)
base::ScopedFD v8_natives_fd_;
base::ScopedFD v8_snapshot_fd_;
#endif // OS_POSIX && !OS_MACOSX

// Vector of additional ChromeContentBrowserClientParts.
// Parts are deleted in the reverse order they are added.
std::vector<ChromeContentBrowserClientParts*> extra_parts_;
Expand Down
57 changes: 31 additions & 26 deletions content/app/content_main_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@
#include "gin/public/isolate_holder.h"
#endif

#if defined(OS_ANDROID)
#include "content/public/common/content_descriptors.h"
#endif

#if defined(USE_TCMALLOC)
#include "third_party/tcmalloc/chromium/src/gperftools/malloc_extension.h"
#if defined(TYPE_PROFILING)
Expand Down Expand Up @@ -98,6 +94,7 @@
#include "content/public/common/content_descriptors.h"

#if !defined(OS_MACOSX)
#include "content/public/common/content_descriptors.h"
#include "content/public/common/zygote_fork_delegate_linux.h"
#endif
#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
Expand Down Expand Up @@ -498,6 +495,10 @@ class ContentMainRunnerImpl : public ContentMainRunner {
}
#endif // !OS_MACOSX && USE_TCMALLOC

#if !defined(OS_IOS)
base::GlobalDescriptors* g_fds = base::GlobalDescriptors::GetInstance();
#endif

// On Android,
// - setlocale() is not supported.
// - We do not override the signal handlers so that we can get
Expand All @@ -510,16 +511,15 @@ class ContentMainRunnerImpl : public ContentMainRunner {
setlocale(LC_ALL, "");

SetupSignalHandlers();

base::GlobalDescriptors* g_fds = base::GlobalDescriptors::GetInstance();
g_fds->Set(kPrimaryIPCChannel,
kPrimaryIPCChannel + base::GlobalDescriptors::kBaseDescriptor);
#endif // !OS_ANDROID && !OS_IOS

#if defined(OS_LINUX) || defined(OS_OPENBSD)
g_fds->Set(kCrashDumpSignal,
kCrashDumpSignal + base::GlobalDescriptors::kBaseDescriptor);
#endif
#endif // OS_LINUX || OS_OPENBSD


#endif // !OS_WIN

Expand Down Expand Up @@ -679,43 +679,48 @@ class ContentMainRunnerImpl : public ContentMainRunner {
RegisterContentSchemes(true);

#if defined(OS_ANDROID)
int icudata_fd = base::GlobalDescriptors::GetInstance()->MaybeGet(
kAndroidICUDataDescriptor);
int icudata_fd = g_fds->MaybeGet(kAndroidICUDataDescriptor);
if (icudata_fd != -1) {
auto icudata_region = base::GlobalDescriptors::GetInstance()->GetRegion(
kAndroidICUDataDescriptor);
auto icudata_region = g_fds->GetRegion(kAndroidICUDataDescriptor);
CHECK(base::i18n::InitializeICUWithFileDescriptor(icudata_fd,
icudata_region));
} else {
CHECK(base::i18n::InitializeICU());
}
#else
CHECK(base::i18n::InitializeICU());
#endif // OS_ANDROID

#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
int v8_natives_fd = base::GlobalDescriptors::GetInstance()->MaybeGet(
kV8NativesDataDescriptor);
int v8_snapshot_fd = base::GlobalDescriptors::GetInstance()->MaybeGet(
kV8SnapshotDataDescriptor);
#if defined(OS_POSIX) && !defined(OS_MACOSX)
#if !defined(OS_ANDROID)
// kV8NativesDataDescriptor and kV8SnapshotDataDescriptor are shared with
// child processes. On Android they are set in
// ChildProcessService::InternalInitChildProcess, otherwise set them here.
if (!process_type.empty() && process_type != switches::kZygoteProcess) {
g_fds->Set(
kV8NativesDataDescriptor,
kV8NativesDataDescriptor + base::GlobalDescriptors::kBaseDescriptor);
g_fds->Set(
kV8SnapshotDataDescriptor,
kV8SnapshotDataDescriptor + base::GlobalDescriptors::kBaseDescriptor);
}
#endif // !OS_ANDROID
int v8_natives_fd = g_fds->MaybeGet(kV8NativesDataDescriptor);
int v8_snapshot_fd = g_fds->MaybeGet(kV8SnapshotDataDescriptor);
if (v8_natives_fd != -1 && v8_snapshot_fd != -1) {
auto v8_natives_region =
base::GlobalDescriptors::GetInstance()->GetRegion(
kV8NativesDataDescriptor);
auto v8_snapshot_region =
base::GlobalDescriptors::GetInstance()->GetRegion(
kV8SnapshotDataDescriptor);
auto v8_natives_region = g_fds->GetRegion(kV8NativesDataDescriptor);
auto v8_snapshot_region = g_fds->GetRegion(kV8SnapshotDataDescriptor);
CHECK(gin::IsolateHolder::LoadV8SnapshotFd(
v8_natives_fd, v8_natives_region.offset, v8_natives_region.size,
v8_snapshot_fd, v8_snapshot_region.offset, v8_snapshot_region.size));
} else {
CHECK(gin::IsolateHolder::LoadV8Snapshot());
}
#endif // V8_USE_EXTERNAL_STARTUP_DATA

#else
CHECK(base::i18n::InitializeICU());
#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
CHECK(gin::IsolateHolder::LoadV8Snapshot());
#endif // OS_POSIX && !OS_MACOSX
#endif // V8_USE_EXTERNAL_STARTUP_DATA
#endif // OS_ANDROID

if (delegate_)
delegate_->PreSandboxStartup();
Expand Down
9 changes: 4 additions & 5 deletions content/public/common/content_descriptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@ enum {
kCrashDumpSignal = kIPCDescriptorMax,
kSandboxIPCChannel, // http://code.google.com/p/chromium/LinuxSandboxIPC

#if defined(OS_ANDROID)
kAndroidPropertyDescriptor,
kAndroidICUDataDescriptor,

#ifdef V8_USE_EXTERNAL_STARTUP_DATA
#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
kV8NativesDataDescriptor,
kV8SnapshotDataDescriptor,
#endif

#if defined(OS_ANDROID)
kAndroidPropertyDescriptor,
kAndroidICUDataDescriptor,
#endif

// The first key that embedders can use to register descriptors (see
Expand Down
1 change: 1 addition & 0 deletions content/shell/DEPS
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
include_rules = [
"+gin/public",
"+v8/include",

# For chromeos build config
Expand Down
30 changes: 29 additions & 1 deletion content/shell/browser/shell_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "content/shell/common/shell_messages.h"
#include "content/shell/common/shell_switches.h"
#include "content/shell/common/webkit_test_helpers.h"
#include "gin/public/isolate_holder.h"
#include "net/url_request/url_request_context_getter.h"
#include "url/gurl.h"

Expand Down Expand Up @@ -127,7 +128,12 @@ void ShellContentBrowserClient::SetSwapProcessesForRedirect(bool swap) {
}

ShellContentBrowserClient::ShellContentBrowserClient()
: shell_browser_main_parts_(NULL) {
:
#if defined(OS_POSIX) && !defined(OS_MACOSX)
v8_natives_fd_(-1),
v8_snapshot_fd_(-1),
#endif // OS_POSIX && !OS_MACOSX
shell_browser_main_parts_(NULL) {
DCHECK(!g_browser_client);
g_browser_client = this;
}
Expand Down Expand Up @@ -317,6 +323,28 @@ void ShellContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
const base::CommandLine& command_line,
int child_process_id,
FileDescriptorInfo* mappings) {
#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
if (v8_snapshot_fd_.get() == -1 && v8_natives_fd_.get() == -1) {
base::FilePath v8_data_path;
PathService::Get(gin::IsolateHolder::kV8SnapshotBasePathKey, &v8_data_path);
DCHECK(!v8_data_path.empty());

int file_flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
base::FilePath v8_natives_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kNativesFileName);
base::FilePath v8_snapshot_data_path =
v8_data_path.AppendASCII(gin::IsolateHolder::kSnapshotFileName);
base::File v8_natives_data_file(v8_natives_data_path, file_flags);
base::File v8_snapshot_data_file(v8_snapshot_data_path, file_flags);
DCHECK(v8_natives_data_file.IsValid());
DCHECK(v8_snapshot_data_file.IsValid());
v8_natives_fd_.reset(v8_natives_data_file.TakePlatformFile());
v8_snapshot_fd_.reset(v8_snapshot_data_file.TakePlatformFile());
}
mappings->Share(kV8NativesDataDescriptor, v8_natives_fd_.get());
mappings->Share(kV8SnapshotDataDescriptor, v8_snapshot_fd_.get());
#endif // V8_USE_EXTERNAL_STARTUP_DATA

#if defined(OS_ANDROID)
int flags = base::File::FLAG_OPEN | base::File::FLAG_READ;
base::FilePath pak_file;
Expand Down
5 changes: 5 additions & 0 deletions content/shell/browser/shell_content_browser_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class ShellContentBrowserClient : public ContentBrowserClient {
scoped_ptr<ShellResourceDispatcherHostDelegate>
resource_dispatcher_host_delegate_;

#if defined(OS_POSIX) && !defined(OS_MACOSX)
base::ScopedFD v8_natives_fd_;
base::ScopedFD v8_snapshot_fd_;
#endif

ShellBrowserMainParts* shell_browser_main_parts_;
};

Expand Down
25 changes: 12 additions & 13 deletions gin/isolate_holder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,6 @@ bool VerifyV8SnapshotFile(base::MemoryMappedFile* snapshot_file,
return !memcmp(fingerprint, output, sizeof(output));
}
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA

#if !defined(OS_MACOSX)
const int v8_snapshot_dir =
#if defined(OS_ANDROID)
base::DIR_ANDROID_APP_DATA;
#elif defined(OS_POSIX)
base::DIR_EXE;
#elif defined(OS_WIN)
base::DIR_MODULE;
#endif // OS_ANDROID
#endif // !OS_MACOSX

#endif // V8_USE_EXTERNAL_STARTUP_DATA

} // namespace
Expand All @@ -130,6 +118,17 @@ extern const unsigned char g_natives_fingerprint[];
extern const unsigned char g_snapshot_fingerprint[];
#endif // V8_VERIFY_EXTERNAL_STARTUP_DATA

#if !defined(OS_MACOSX)
const int IsolateHolder::kV8SnapshotBasePathKey =
#if defined(OS_ANDROID)
base::DIR_ANDROID_APP_DATA;
#elif defined(OS_POSIX)
base::DIR_EXE;
#elif defined(OS_WIN)
base::DIR_MODULE;
#endif // OS_ANDROID
#endif // !OS_MACOSX

const char IsolateHolder::kNativesFileName[] = "natives_blob.bin";
const char IsolateHolder::kSnapshotFileName[] = "snapshot_blob.bin";

Expand All @@ -140,7 +139,7 @@ bool IsolateHolder::LoadV8Snapshot() {

#if !defined(OS_MACOSX)
base::FilePath data_path;
PathService::Get(v8_snapshot_dir, &data_path);
PathService::Get(kV8SnapshotBasePathKey, &data_path);
DCHECK(!data_path.empty());

base::FilePath natives_path = data_path.AppendASCII(kNativesFileName);
Expand Down
1 change: 1 addition & 0 deletions gin/public/isolate_holder.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class GIN_EXPORT IsolateHolder {
void RemoveRunMicrotasksObserver();

#if defined(V8_USE_EXTERNAL_STARTUP_DATA)
static const int kV8SnapshotBasePathKey;
static const char kNativesFileName[];
static const char kSnapshotFileName[];

Expand Down

0 comments on commit 3fb0727

Please sign in to comment.