chromium side changes to enable nexe exit status to be reported to JavaScript

this CL contains changes to the plugin to propagate the PPAPI event
handler nexe's exit status to JavaScript via an integer exitStatus
property, just like the lastError string property.

this CL depends on NaCl side CL
http://codereview.chromium.org/8139016/ which was committed as NaCl
r6851 and requires DEPS's nacl_revision value to be at least 6851.

BUG= http://code.google.com/p/chromium/issues/detail?id=97833
TEST= exit_status_tests (NaCl-side)
Review URL: http://codereview.chromium.org/8136022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@104578 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 85 insertions, 5 deletions

diff --git a/ppapi/native_client/src/trusted/plugin/plugin.cc b/ppapi/native_client/src/trusted/plugin/plugin.cc
index 8c4924b..1648d2e 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.cc
+++ b/ppapi/native_client/src/trusted/plugin/plugin.cc
@@ -119,6 +119,15 @@ bool GetLastError(void* obj, SrpcParams* params) {
   return true;
 }
 
+bool GetExitStatus(void* obj, SrpcParams* params) {
+  NaClSrpcArg** outs = params->outs();
+  PLUGIN_PRINTF(("GetExitStatus (obj=%p)\n", obj));
+
+  Plugin* plugin = static_cast<Plugin*>(obj);
+  outs[0]->u.ival = plugin->exit_status();
+  return true;
+}
+
 // Up to 20 seconds
 const int64_t kTimeSmallMin = 1;         // in ms
 const int64_t kTimeSmallMax = 20000;     // in ms
@@ -926,6 +935,8 @@ bool Plugin::Init(uint32_t argc, const char* argn[], const char* argv[]) {
 
   // Export a property to allow us to get the last error description.
   AddPropertyGet(GetLastError, "lastError", "s");
+  // Export a property to allow us to get the nexe exit status.
+  AddPropertyGet(GetExitStatus, "exitStatus", "i");
 
   PLUGIN_PRINTF(("Plugin::Init (status=%d)\n", status));
   return status;
@@ -985,11 +996,38 @@ Plugin::~Plugin() {
   ShutdownProxy();
   ScriptableHandle* scriptable_handle_ = scriptable_handle();
   ScriptableHandle::Unref(&scriptable_handle_);
-  NaClSrpcModuleFini();
 
-  // TODO(sehr,polina): We should not need to call ShutDownSubprocesses() here.
+  // ShutDownSubprocesses shuts down the subprocesses, which shuts
+  // down the main ServiceRuntime object, which kills the subprocess.
+  // As a side effect of the subprocess being killed, the reverse
+  // services thread(s) will get EOF on the reverse channel(s), and
+  // the thread(s) will exit.  In ServiceRuntime::Shutdown, we invoke
+  // ReverseService::WaitForServiceThreadsToExit(), so that there will
+  // not be an extent thread(s) hanging around.  This means that the
+  // ~Plugin will block until this happens.  This is a requirement,
+  // since the renderer should be free to unload the plugin code, and
+  // we cannot have threads running code that gets unloaded before
+  // they exit.
+  //
+  // By waiting for the threads here, we also ensure that the Plugin
+  // object and the subprocess and ServiceRuntime objects is not
+  // (fully) destroyed while the threads are running, so resources
+  // that are destroyed after ShutDownSubprocesses (below) are
+  // guaranteed to be live and valid for access from the service
+  // threads.
+  //
+  // The main_subprocess object, which wraps the main service_runtime
+  // object, is dtor'd implicitly after the explicit code below runs,
+  // so the main service runtime object will not have been dtor'd,
+  // though the Shutdown method may have been called, during the
+  // lifetime of the service threads.
   ShutDownSubprocesses();
 
+  // Shutdown Srpc module only after the service threads are done.
+  // NB: NaClSrpcModuleInit and Fini should be invoked at dll
+  // load/unload and not in the Plugin ctor/dtor.
+  NaClSrpcModuleFini();
+
   delete wrapper_factory_;
   delete browser_interface_;
   delete[] argv_;
diff --git a/ppapi/native_client/src/trusted/plugin/plugin.h b/ppapi/native_client/src/trusted/plugin/plugin.h
index 4300c28..5c528a3 100644
--- a/ppapi/native_client/src/trusted/plugin/plugin.h
+++ b/ppapi/native_client/src/trusted/plugin/plugin.h
@@ -316,6 +316,14 @@ class Plugin : public pp::InstancePrivate {
 
   Manifest const* manifest() const { return manifest_.get(); }
 
+  // Extracts the exit status from the (main) service runtime.
+  int exit_status() const {
+    if (NULL == main_service_runtime()) {
+      return -1;
+    }
+    return main_service_runtime()->exit_status();
+  }
+
  private:
   NACL_DISALLOW_COPY_AND_ASSIGN(Plugin);
 #ifndef HACK_FOR_MACOS_HANG_REMOVED
@@ -486,8 +494,8 @@ class Plugin : public pp::InstancePrivate {
   // URL processing interface for use in looking up resources in manifests.
   const pp::URLUtil_Dev* url_util_;
 
-  // A string containing the text description of the last error produced by
-  // this plugin.
+  // A string containing the text description of the last error
+  // produced by this plugin.
   nacl::string last_error_string_;
 
   // A pointer to the browser end of a proxy pattern connecting the
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.cc b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
index 4dab4cc..adf5d7e 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.cc
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.cc
@@ -56,10 +56,12 @@ namespace plugin {
 PluginReverseInterface::PluginReverseInterface(
     nacl::WeakRefAnchor* anchor,
     Plugin* plugin,
+    ServiceRuntime* service_runtime,
     pp::CompletionCallback init_done_cb,
     pp::CompletionCallback crash_cb)
       : anchor_(anchor),
         plugin_(plugin),
+        service_runtime_(service_runtime),
         shutting_down_(false),
         init_done_cb_(init_done_cb),
         crash_cb_(crash_cb) {
@@ -337,6 +339,10 @@ void PluginReverseInterface::ReportCrash() {
   }
 }
 
+void PluginReverseInterface::ReportExitStatus(int exit_status) {
+  service_runtime_->set_exit_status(exit_status);
+}
+
 ServiceRuntime::ServiceRuntime(Plugin* plugin,
                                pp::CompletionCallback init_done_cb,
                                pp::CompletionCallback crash_cb)
@@ -348,8 +354,11 @@ ServiceRuntime::ServiceRuntime(Plugin* plugin,
       async_send_desc_(NULL),
       anchor_(new nacl::WeakRefAnchor()),
       rev_interface_(new PluginReverseInterface(anchor_, plugin,
-                                                init_done_cb, crash_cb)) {
+                                                this,
+                                                init_done_cb, crash_cb)),
+      exit_status_(-1) {
   NaClSrpcChannelInitialize(&command_channel_);
+  NaClXMutexCtor(&mu_);
 }
 
 bool ServiceRuntime::InitCommunication(nacl::DescWrapper* nacl_desc,
@@ -562,6 +571,17 @@ ServiceRuntime::~ServiceRuntime() {
   rev_interface_->Unref();
 
   anchor_->Unref();
+  NaClMutexDtor(&mu_);
+}
+
+int ServiceRuntime::exit_status() {
+  nacl::MutexLocker take(&mu_);
+  return exit_status_;
+}
+
+void ServiceRuntime::set_exit_status(int exit_status) {
+  nacl::MutexLocker take(&mu_);
+  exit_status_ = exit_status & 0xff;
 }
 
 }  // namespace plugin
diff --git a/ppapi/native_client/src/trusted/plugin/service_runtime.h b/ppapi/native_client/src/trusted/plugin/service_runtime.h
index 111747b..2d539e6 100644
--- a/ppapi/native_client/src/trusted/plugin/service_runtime.h
+++ b/ppapi/native_client/src/trusted/plugin/service_runtime.h
@@ -92,6 +92,7 @@ class PluginReverseInterface: public nacl::ReverseInterface {
  public:
   PluginReverseInterface(nacl::WeakRefAnchor* anchor,
                          Plugin* plugin,
+                         ServiceRuntime* service_runtime,
                          pp::CompletionCallback init_done_cb,
                          pp::CompletionCallback crash_cb);
 
@@ -111,6 +112,8 @@ class PluginReverseInterface: public nacl::ReverseInterface {
 
   virtual void ReportCrash();
 
+  virtual void ReportExitStatus(int exit_status);
+
  protected:
   virtual void Log_MainThreadContinuation(LogToJavaScriptConsoleResource* p,
                                           int32_t err);
@@ -131,6 +134,7 @@ class PluginReverseInterface: public nacl::ReverseInterface {
   nacl::WeakRefAnchor* anchor_;  // holds a ref
   Plugin* plugin_;  // value may be copied, but should be used only in
                     // main thread in WeakRef-protected callbacks.
+  ServiceRuntime* service_runtime_;
   NaClMutex mu_;
   NaClCondVar cv_;
   bool shutting_down_;
@@ -164,6 +168,13 @@ class ServiceRuntime {
   Plugin* plugin() const { return plugin_; }
   void Shutdown();
 
+  // exit_status is -1 when invalid; when we set it, we will ensure
+  // that it is non-negative (the portion of the exit status from the
+  // nexe that is transferred is the low 8 bits of the argument to the
+  // exit syscall).
+  int exit_status();  // const, but grabs mutex etc.
+  void set_exit_status(int exit_status);
+
   nacl::DescWrapper* async_receive_desc() { return async_receive_desc_.get(); }
   nacl::DescWrapper* async_send_desc() { return async_send_desc_.get(); }
 
@@ -187,6 +198,9 @@ class ServiceRuntime {
   nacl::WeakRefAnchor* anchor_;
 
   PluginReverseInterface* rev_interface_;
+
+  NaClMutex mu_;
+  int exit_status_;
 };
 
 }  // namespace plugin