diff options
-rw-r--r-- | chrome/test/nacl/nacl_browsertest.cc | 3 | ||||
-rw-r--r-- | components/nacl/renderer/ppb_nacl_private_impl.cc | 7 | ||||
-rw-r--r-- | components/nacl/renderer/trusted_plugin_channel.cc | 5 | ||||
-rw-r--r-- | components/nacl/renderer/trusted_plugin_channel.h | 1 | ||||
-rw-r--r-- | ppapi/api/private/ppb_nacl_private.idl | 3 | ||||
-rw-r--r-- | ppapi/c/private/ppb_nacl_private.h | 4 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/plugin.cc | 18 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/plugin.h | 15 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/service_runtime.cc | 49 | ||||
-rw-r--r-- | ppapi/native_client/src/trusted/plugin/service_runtime.h | 9 | ||||
-rw-r--r-- | ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c | 6 |
11 files changed, 30 insertions, 90 deletions
diff --git a/chrome/test/nacl/nacl_browsertest.cc b/chrome/test/nacl/nacl_browsertest.cc index 7e26901..1e17317 100644 --- a/chrome/test/nacl/nacl_browsertest.cc +++ b/chrome/test/nacl/nacl_browsertest.cc @@ -107,9 +107,6 @@ IN_PROC_BROWSER_TEST_F(NaClBrowserTestNewlib, BadNative) { #if defined(OS_WIN) // crbug.com/98721 # define MAYBE_Crash DISABLED_Crash -#elif defined(OS_LINUX) -// crbug.com/366334 -# define MAYBE_Crash DISABLED_Crash #else # define MAYBE_Crash Crash #endif diff --git a/components/nacl/renderer/ppb_nacl_private_impl.cc b/components/nacl/renderer/ppb_nacl_private_impl.cc index 58faad7..de005df 100644 --- a/components/nacl/renderer/ppb_nacl_private_impl.cc +++ b/components/nacl/renderer/ppb_nacl_private_impl.cc @@ -749,12 +749,6 @@ void ReportLoadAbort(PP_Instance instance) { load_manager->ReportLoadAbort(); } -void NexeDidCrash(PP_Instance instance) { - NexeLoadManager* load_manager = NexeLoadManager::Get(instance); - if (load_manager) - load_manager->NexeDidCrash(); -} - void InstanceCreated(PP_Instance instance) { NexeLoadManager::Create(instance); } @@ -1666,7 +1660,6 @@ const PPB_NaCl_Private nacl_interface = { &ReportLoadSuccess, &ReportLoadError, &ReportLoadAbort, - &NexeDidCrash, &InstanceCreated, &InstanceDestroyed, &NaClDebugEnabledForURL, diff --git a/components/nacl/renderer/trusted_plugin_channel.cc b/components/nacl/renderer/trusted_plugin_channel.cc index 0aaf6bc..729644c 100644 --- a/components/nacl/renderer/trusted_plugin_channel.cc +++ b/components/nacl/renderer/trusted_plugin_channel.cc @@ -46,6 +46,11 @@ bool TrustedPluginChannel::OnMessageReceived(const IPC::Message& msg) { return handled; } +void TrustedPluginChannel::OnChannelError() { + if (report_exit_status_) + nexe_load_manager_->NexeDidCrash(); +} + void TrustedPluginChannel::OnReportExitStatus(int exit_status) { if (report_exit_status_) nexe_load_manager_->set_exit_status(exit_status); diff --git a/components/nacl/renderer/trusted_plugin_channel.h b/components/nacl/renderer/trusted_plugin_channel.h index fd261aa..5f49f12 100644 --- a/components/nacl/renderer/trusted_plugin_channel.h +++ b/components/nacl/renderer/trusted_plugin_channel.h @@ -35,6 +35,7 @@ class TrustedPluginChannel : public IPC::Listener { // Listener implementation. virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE; + virtual void OnChannelError() OVERRIDE; void OnReportExitStatus(int exit_status); diff --git a/ppapi/api/private/ppb_nacl_private.idl b/ppapi/api/private/ppb_nacl_private.idl index e777527..3924ca2 100644 --- a/ppapi/api/private/ppb_nacl_private.idl +++ b/ppapi/api/private/ppb_nacl_private.idl @@ -275,9 +275,6 @@ interface PPB_NaCl_Private { /* Reports that loading a nexe was aborted. */ void ReportLoadAbort([in] PP_Instance instance); - /* Reports that the nexe has crashed. */ - void NexeDidCrash([in] PP_Instance instance); - /* Performs internal setup when an instance is created. */ void InstanceCreated([in] PP_Instance instance); diff --git a/ppapi/c/private/ppb_nacl_private.h b/ppapi/c/private/ppb_nacl_private.h index b28dff2..5ba08d9 100644 --- a/ppapi/c/private/ppb_nacl_private.h +++ b/ppapi/c/private/ppb_nacl_private.h @@ -3,7 +3,7 @@ * found in the LICENSE file. */ -/* From private/ppb_nacl_private.idl modified Wed Aug 27 13:55:13 2014. */ +/* From private/ppb_nacl_private.idl modified Thu Sep 4 07:46:02 2014. */ #ifndef PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_ #define PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_ @@ -304,8 +304,6 @@ struct PPB_NaCl_Private_1_0 { const char* error_message); /* Reports that loading a nexe was aborted. */ void (*ReportLoadAbort)(PP_Instance instance); - /* Reports that the nexe has crashed. */ - void (*NexeDidCrash)(PP_Instance instance); /* Performs internal setup when an instance is created. */ void (*InstanceCreated)(PP_Instance instance); /* Performs internal cleanup when an instance is destroyed. */ diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc index ea86558..e7b2568 100644 --- a/ppapi/native_client/src/trusted/plugin/plugin.cc +++ b/ppapi/native_client/src/trusted/plugin/plugin.cc @@ -72,7 +72,6 @@ bool Plugin::LoadHelperNaClModuleInternal(NaClSubprocess* subprocess, pp_instance(), false, // No main_service_runtime. false, // No non-SFI mode (i.e. in SFI-mode). - pp::BlockUntilComplete(), pp::BlockUntilComplete()); subprocess->set_service_runtime(service_runtime); @@ -128,8 +127,7 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info, bool enable_dyncode_syscalls, bool enable_exception_handling, bool enable_crash_throttling, - const pp::CompletionCallback& init_done_cb, - const pp::CompletionCallback& crash_cb) { + const pp::CompletionCallback& init_done_cb) { CHECK(pp::Module::Get()->core()->IsMainThread()); // Before forking a new sel_ldr process, ensure that we do not leak // the ServiceRuntime object for an existing subprocess, and that any @@ -149,7 +147,7 @@ void Plugin::LoadNaClModule(PP_NaClFileInfo file_info, enable_crash_throttling); ErrorInfo error_info; ServiceRuntime* service_runtime = new ServiceRuntime( - this, pp_instance(), true, uses_nonsfi_mode, init_done_cb, crash_cb); + this, pp_instance(), true, uses_nonsfi_mode, init_done_cb); main_subprocess_.set_service_runtime(service_runtime); if (NULL == service_runtime) { error_info.SetReport( @@ -336,8 +334,7 @@ void Plugin::NexeFileDidOpen(int32_t pp_error) { true, /* enable_dyncode_syscalls */ true, /* enable_exception_handling */ false, /* enable_crash_throttling */ - callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation), - callback_factory_.NewCallback(&Plugin::NexeDidCrash)); + callback_factory_.NewCallback(&Plugin::NexeFileDidOpenContinuation)); } void Plugin::NexeFileDidOpenContinuation(int32_t pp_error) { @@ -355,12 +352,6 @@ void Plugin::NexeFileDidOpenContinuation(int32_t pp_error) { NaClLog(4, "Leaving NexeFileDidOpenContinuation\n"); } -void Plugin::NexeDidCrash(int32_t pp_error) { - PLUGIN_PRINTF(("Plugin::NexeDidCrash (pp_error=%" NACL_PRId32 ")\n", - pp_error)); - nacl_interface_->NexeDidCrash(pp_instance()); -} - void Plugin::BitcodeDidTranslate(int32_t pp_error) { PLUGIN_PRINTF(("Plugin::BitcodeDidTranslate (pp_error=%" NACL_PRId32 ")\n", pp_error)); @@ -382,8 +373,7 @@ void Plugin::BitcodeDidTranslate(int32_t pp_error) { false, /* enable_dyncode_syscalls */ false, /* enable_exception_handling */ true, /* enable_crash_throttling */ - callback_factory_.NewCallback(&Plugin::BitcodeDidTranslateContinuation), - callback_factory_.NewCallback(&Plugin::NexeDidCrash)); + callback_factory_.NewCallback(&Plugin::BitcodeDidTranslateContinuation)); } void Plugin::BitcodeDidTranslateContinuation(int32_t pp_error) { diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h index a55b62d..8b89a88 100644 --- a/ppapi/native_client/src/trusted/plugin/plugin.h +++ b/ppapi/native_client/src/trusted/plugin/plugin.h @@ -93,8 +93,7 @@ class Plugin : public pp::Instance { bool enable_dyncode_syscalls, bool enable_exception_handling, bool enable_crash_throttling, - const pp::CompletionCallback& init_done_cb, - const pp::CompletionCallback& crash_cb); + const pp::CompletionCallback& init_done_cb); // Finish hooking interfaces up, after low-level initialization is // complete. @@ -173,18 +172,6 @@ class Plugin : public pp::Instance { void NexeFileDidOpen(int32_t pp_error); void NexeFileDidOpenContinuation(int32_t pp_error); - // Callback used when the reverse channel closes. This is an - // asynchronous event that might turn into a JavaScript error or - // crash event -- this is controlled by the two state variables - // nacl_ready_state_ and nexe_error_reported_: If an error or crash - // had already been reported, no additional crash event is - // generated. If no error has been reported but nacl_ready_state_ - // is not DONE, then the loadend event has not been reported, and we - // enqueue an error event followed by loadend. If nacl_ready_state_ - // is DONE, then we are in the post-loadend (we need temporal - // predicate symbols), and we enqueue a crash event. - void NexeDidCrash(int32_t pp_error); - // Callback used when a .nexe is translated from bitcode. If the translation // is successful, the file descriptor is opened and can be passed to sel_ldr // with the sandbox on. diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc index b162f62..4787162 100644 --- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc +++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc @@ -58,14 +58,12 @@ PluginReverseInterface::PluginReverseInterface( nacl::WeakRefAnchor* anchor, PP_Instance pp_instance, ServiceRuntime* service_runtime, - pp::CompletionCallback init_done_cb, - pp::CompletionCallback crash_cb) + pp::CompletionCallback init_done_cb) : anchor_(anchor), pp_instance_(pp_instance), service_runtime_(service_runtime), shutting_down_(false), - init_done_cb_(init_done_cb), - crash_cb_(crash_cb) { + init_done_cb_(init_done_cb) { NaClXMutexCtor(&mu_); NaClXCondVarCtor(&cv_); } @@ -222,18 +220,7 @@ void PluginReverseInterface::StreamAsFile_MainThreadContinuation( } void PluginReverseInterface::ReportCrash() { - NaClLog(4, "PluginReverseInterface::ReportCrash\n"); - - if (crash_cb_.pp_completion_callback().func != NULL) { - NaClLog(4, "PluginReverseInterface::ReportCrash: invoking CB\n"); - pp::Module::Get()->core()->CallOnMainThread(0, crash_cb_, PP_OK); - // Clear the callback to avoid it gets invoked twice. - crash_cb_ = pp::CompletionCallback(); - } else { - NaClLog(1, - "PluginReverseInterface::ReportCrash:" - " crash_cb_ not valid, skipping\n"); - } + // This is now handled through Chromium IPC. } void PluginReverseInterface::ReportExitStatus(int exit_status) { @@ -250,8 +237,7 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin, PP_Instance pp_instance, bool main_service_runtime, bool uses_nonsfi_mode, - pp::CompletionCallback init_done_cb, - pp::CompletionCallback crash_cb) + pp::CompletionCallback init_done_cb) : plugin_(plugin), pp_instance_(pp_instance), main_service_runtime_(main_service_runtime), @@ -259,7 +245,7 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin, reverse_service_(NULL), anchor_(new nacl::WeakRefAnchor()), rev_interface_(new PluginReverseInterface(anchor_, pp_instance, this, - init_done_cb, crash_cb)), + init_done_cb)), start_sel_ldr_done_(false), start_nexe_done_(false), nexe_started_ok_(false), @@ -487,24 +473,21 @@ bool ServiceRuntime::StartNexeInternal() { } void ServiceRuntime::ReapLogs() { - // On a load failure the service runtime does not crash itself to + // TODO(teravest): We should allow the NaCl process to crash itself when a + // module fails to start, and remove the call to RemoteLog() here. The + // reverse channel is no longer needed for crash reporting. + // + // The reasoning behind the current code behavior follows: + // On a load failure the NaCl process does not crash itself to // avoid a race where the no-more-senders error on the reverse // channel service thread might cause the crash-detection logic to // kick in before the start_module RPC reply has been received. So - // we induce a service runtime crash here. We do not release - // subprocess_ since it's needed to collect crash log output after - // the error is reported. + // we induce a NaCl process crash here. RemoteLog(LOG_FATAL, "reap logs\n"); - if (NULL == reverse_service_) { - // No crash detector thread. - NaClLog(LOG_ERROR, "scheduling to get crash log\n"); - // Invoking rev_interface's method is workaround to avoid crash_cb - // gets called twice or more. We should clean this up later. - rev_interface_->ReportCrash(); - NaClLog(LOG_ERROR, "should fire soon\n"); - } else { - NaClLog(LOG_ERROR, "Reverse service thread will pick up crash log\n"); - } + + // TODO(teravest): Release subprocess_ here since it's no longer needed. It + // was previously kept around to collect crash log output from the bootstrap + // channel. } void ServiceRuntime::ReportLoadError(const ErrorInfo& error_info) { diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h index 2b92727..2fccc7e 100644 --- a/ppapi/native_client/src/trusted/plugin/service_runtime.h +++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h @@ -96,8 +96,7 @@ class PluginReverseInterface: public nacl::ReverseInterface { PluginReverseInterface(nacl::WeakRefAnchor* anchor, PP_Instance pp_instance, ServiceRuntime* service_runtime, - pp::CompletionCallback init_done_cb, - pp::CompletionCallback crash_cb); + pp::CompletionCallback init_done_cb); virtual ~PluginReverseInterface(); @@ -139,20 +138,16 @@ class PluginReverseInterface: public nacl::ReverseInterface { bool shutting_down_; pp::CompletionCallback init_done_cb_; - pp::CompletionCallback crash_cb_; }; // ServiceRuntime abstracts a NativeClient sel_ldr instance. class ServiceRuntime { public: - // TODO(sehr): This class should also implement factory methods, using the - // Start method below. ServiceRuntime(Plugin* plugin, PP_Instance pp_instance, bool main_service_runtime, bool uses_nonsfi_mode, - pp::CompletionCallback init_done_cb, - pp::CompletionCallback crash_cb); + pp::CompletionCallback init_done_cb); // The destructor terminates the sel_ldr process. ~ServiceRuntime(); diff --git a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c index 73c18d8..ab1beda 100644 --- a/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c +++ b/ppapi/native_client/src/untrusted/pnacl_irt_shim/pnacl_shim.c @@ -3492,11 +3492,6 @@ static void Pnacl_M25_PPB_NaCl_Private_ReportLoadAbort(PP_Instance instance) { iface->ReportLoadAbort(instance); } -static void Pnacl_M25_PPB_NaCl_Private_NexeDidCrash(PP_Instance instance) { - const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface; - iface->NexeDidCrash(instance); -} - static void Pnacl_M25_PPB_NaCl_Private_InstanceCreated(PP_Instance instance) { const struct PPB_NaCl_Private_1_0 *iface = Pnacl_WrapperInfo_PPB_NaCl_Private_1_0.real_iface; iface->InstanceCreated(instance); @@ -5408,7 +5403,6 @@ static const struct PPB_NaCl_Private_1_0 Pnacl_Wrappers_PPB_NaCl_Private_1_0 = { .ReportLoadSuccess = (void (*)(PP_Instance instance, uint64_t loaded_bytes, uint64_t total_bytes))&Pnacl_M25_PPB_NaCl_Private_ReportLoadSuccess, .ReportLoadError = (void (*)(PP_Instance instance, PP_NaClError error, const char* error_message))&Pnacl_M25_PPB_NaCl_Private_ReportLoadError, .ReportLoadAbort = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_ReportLoadAbort, - .NexeDidCrash = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_NexeDidCrash, .InstanceCreated = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceCreated, .InstanceDestroyed = (void (*)(PP_Instance instance))&Pnacl_M25_PPB_NaCl_Private_InstanceDestroyed, .NaClDebugEnabledForURL = (PP_Bool (*)(const char* alleged_nmf_url))&Pnacl_M25_PPB_NaCl_Private_NaClDebugEnabledForURL, |