From 4d65db8a959b5efeedc77682a21da1b646ae383f Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Mon, 31 Oct 2016 15:56:25 -0700 Subject: [PATCH] Don't use IsolateData for the exit hook as multiple embedders share the dart/bin while using different isolate data structures. Issue flutter/flutter#6506 R=asiva@google.com Review URL: https://codereview.chromium.org/2463923002 . --- runtime/bin/isolate_data.h | 9 +-------- runtime/bin/main.cc | 15 ++++++++++++--- runtime/bin/process.cc | 6 +----- runtime/bin/process.h | 11 +++++++++++ runtime/bin/process_android.cc | 1 + runtime/bin/process_fuchsia.cc | 1 + runtime/bin/process_linux.cc | 1 + runtime/bin/process_macos.cc | 1 + runtime/bin/process_unsupported.cc | 1 + runtime/bin/process_win.cc | 1 + 10 files changed, 31 insertions(+), 16 deletions(-) diff --git a/runtime/bin/isolate_data.h b/runtime/bin/isolate_data.h index fe76e51d89f..5ed1ed864f6 100644 --- a/runtime/bin/isolate_data.h +++ b/runtime/bin/isolate_data.h @@ -16,8 +16,6 @@ namespace bin { class EventHandler; class Loader; -typedef void (*ExitHook)(int64_t exit_code); - // Data associated with every isolate in the standalone VM // embedding. This is used to free external resources for each isolate // when the isolate shuts down. @@ -31,8 +29,7 @@ class IsolateData { packages_file(NULL), udp_receive_buffer(NULL), builtin_lib_(NULL), - loader_(NULL), - exit_hook_(NULL) { + loader_(NULL) { if (package_root != NULL) { ASSERT(packages_file == NULL); this->package_root = strdup(package_root); @@ -67,9 +64,6 @@ class IsolateData { builtin_lib_ = Dart_NewPersistentHandle(lib); } - ExitHook exit_hook() const { return exit_hook_; } - void set_exit_hook(ExitHook hook) { exit_hook_ = hook; } - char* script_url; char* package_root; char* packages_file; @@ -89,7 +83,6 @@ class IsolateData { private: Dart_Handle builtin_lib_; Loader* loader_; - ExitHook exit_hook_; DISALLOW_COPY_AND_ASSIGN(IsolateData); }; diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc index c953b625dfe..773fdda2eaa 100644 --- a/runtime/bin/main.cc +++ b/runtime/bin/main.cc @@ -105,6 +105,9 @@ extern const char* kPrecompiledDataSymbolName; static bool trace_loading = false; +static Dart_Isolate main_isolate = NULL; + + static const char* DEFAULT_VM_SERVICE_SERVER_IP = "127.0.0.1"; static const int DEFAULT_VM_SERVICE_SERVER_PORT = 8181; // VM Service options. @@ -798,9 +801,6 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri, IsolateData* isolate_data = new IsolateData(script_uri, package_root, packages_config); - if (gen_snapshot_kind == kAppJIT) { - isolate_data->set_exit_hook(SnapshotOnExitHook); - } Dart_Isolate isolate = Dart_CreateIsolate(script_uri, main, isolate_snapshot_buffer, @@ -1539,6 +1539,11 @@ static void GenerateAppSnapshot() { static void SnapshotOnExitHook(int64_t exit_code) { + if (Dart_CurrentIsolate() != main_isolate) { + Log::PrintErr("A snapshot was requested, but a secondary isolate " + "performed a hard exit (%" Pd64 ").\n", exit_code); + Platform::Exit(kErrorExitCode); + } if (exit_code == 0) { GenerateAppSnapshot(); } @@ -1577,6 +1582,7 @@ bool RunMainIsolate(const char* script_name, EventHandler::Stop(); Platform::Exit((exit_code != 0) ? exit_code : kErrorExitCode); } + main_isolate = isolate; delete [] isolate_name; Dart_EnterIsolate(isolate); @@ -1891,6 +1897,9 @@ void main(int argc, char** argv) { #if defined(DART_PRECOMPILED_RUNTIME) vm_options.AddArgument("--precompilation"); #endif + if (gen_snapshot_kind == kAppJIT) { + Process::SetExitHook(SnapshotOnExitHook); + } Dart_SetVMFlags(vm_options.count(), vm_options.arguments()); diff --git a/runtime/bin/process.cc b/runtime/bin/process.cc index ff88201bc12..e22e07c4fab 100644 --- a/runtime/bin/process.cc +++ b/runtime/bin/process.cc @@ -246,11 +246,7 @@ void FUNCTION_NAME(Process_Exit)(Dart_NativeArguments args) { int64_t status = 0; // Ignore result if passing invalid argument and just exit 0. DartUtils::GetInt64Value(Dart_GetNativeArgument(args, 0), &status); - IsolateData* isolate_data = - reinterpret_cast(Dart_CurrentIsolateData()); - if (isolate_data->exit_hook() != NULL) { - isolate_data->exit_hook()(status); - } + Process::RunExitHook(status); Dart_ExitIsolate(); Platform::Exit(static_cast(status)); } diff --git a/runtime/bin/process.h b/runtime/bin/process.h index 2886e71d7c8..4ef892908d8 100644 --- a/runtime/bin/process.h +++ b/runtime/bin/process.h @@ -126,6 +126,16 @@ class Process { global_exit_code_ = exit_code; } + typedef void (*ExitHook)(int64_t exit_code); + static void SetExitHook(ExitHook hook) { + exit_hook_ = hook; + } + static void RunExitHook(int64_t exit_code) { + if (exit_hook_ != NULL) { + exit_hook_(exit_code); + } + } + static intptr_t CurrentProcessId(); static intptr_t SetSignalHandler(intptr_t signal); @@ -139,6 +149,7 @@ class Process { private: static int global_exit_code_; static Mutex* global_exit_code_mutex_; + static ExitHook exit_hook_; DISALLOW_ALLOCATION(); DISALLOW_IMPLICIT_CONSTRUCTORS(Process); diff --git a/runtime/bin/process_android.cc b/runtime/bin/process_android.cc index 00d6ca2df8d..89c5bf7d599 100644 --- a/runtime/bin/process_android.cc +++ b/runtime/bin/process_android.cc @@ -34,6 +34,7 @@ namespace bin { int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; // ProcessInfo is used to map a process id to the file descriptor for // the pipe used to communicate the exit code of the process to Dart. diff --git a/runtime/bin/process_fuchsia.cc b/runtime/bin/process_fuchsia.cc index 5858800aba0..cd2f8bfb191 100644 --- a/runtime/bin/process_fuchsia.cc +++ b/runtime/bin/process_fuchsia.cc @@ -18,6 +18,7 @@ namespace bin { int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; void Process::TerminateExitCodeHandler() { } diff --git a/runtime/bin/process_linux.cc b/runtime/bin/process_linux.cc index e533cc50d65..fff03f9a258 100644 --- a/runtime/bin/process_linux.cc +++ b/runtime/bin/process_linux.cc @@ -34,6 +34,7 @@ namespace bin { int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; // ProcessInfo is used to map a process id to the file descriptor for // the pipe used to communicate the exit code of the process to Dart. diff --git a/runtime/bin/process_macos.cc b/runtime/bin/process_macos.cc index b3b5662474f..9f671a8bde5 100644 --- a/runtime/bin/process_macos.cc +++ b/runtime/bin/process_macos.cc @@ -35,6 +35,7 @@ namespace bin { int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; // ProcessInfo is used to map a process id to the file descriptor for // the pipe used to communicate the exit code of the process to Dart. diff --git a/runtime/bin/process_unsupported.cc b/runtime/bin/process_unsupported.cc index 64bed269e10..d91b83c1532 100644 --- a/runtime/bin/process_unsupported.cc +++ b/runtime/bin/process_unsupported.cc @@ -15,6 +15,7 @@ namespace bin { int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; void Process::TerminateExitCodeHandler() { } diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc index b96e67bf294..c3737877cce 100644 --- a/runtime/bin/process_win.cc +++ b/runtime/bin/process_win.cc @@ -29,6 +29,7 @@ static const int kWriteHandle = 1; int Process::global_exit_code_ = 0; Mutex* Process::global_exit_code_mutex_ = new Mutex(); +Process::ExitHook Process::exit_hook_ = NULL; // ProcessInfo is used to map a process id to the process handle, // wait handle for registered exit code event and the pipe used to