Open NaCl IRT file only once at startup

This changes the NaCl startup dance so that the IRT file is opened
just once at browser startup.  That file descriptor is kept around and
passed repeatedly to each NaCl process launched.  This ensures that
when autoupdate replaces the file on disk with a new version, we
continue to use the original file that corresponds to the old browser
version that's still running.

We also eliminate the cases for not having an IRT file, which is now a
hard error (i.e. prevents NaCl launches).  It's been a hard
requirement for NaCl that the IRT be available since Chromium 14.

BUG= http://code.google.com/p/nativeclient/issues/detail?id=1772
TEST= hand-tested in Chromium build on Linux, Mac, and Windows

R=brettw@chromium.org

Review URL: http://codereview.chromium.org/8397001

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

8 files changed, 225 insertions, 103 deletions

diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 432b505..1071fd0 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -50,6 +50,7 @@
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/metrics/thread_watcher.h"
 #include "chrome/browser/metrics/tracking_synchronizer.h"
+#include "chrome/browser/nacl_host/nacl_process_host.h"
 #include "chrome/browser/net/chrome_dns_cert_provenance_checker.h"
 #include "chrome/browser/net/chrome_dns_cert_provenance_checker_factory.h"
 #include "chrome/browser/net/chrome_net_log.h"
@@ -1843,6 +1844,10 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   // Start watching all browser threads for responsiveness.
   ThreadWatcherList::StartWatchingAll(parsed_command_line());
 
+#if !defined(DISABLE_NACL)
+  NaClProcessHost::EarlyStartup();
+#endif
+
   run_message_loop_ = true;
   return content::RESULT_CODE_NORMAL_EXIT;
 }
diff --git a/chrome/browser/nacl_host/nacl_process_host.cc b/chrome/browser/nacl_host/nacl_process_host.cc
index 62a1f41..d254d44 100644
--- a/chrome/browser/nacl_host/nacl_process_host.cc
+++ b/chrome/browser/nacl_host/nacl_process_host.cc
@@ -40,13 +40,59 @@ namespace {
 void SetCloseOnExec(nacl::Handle fd) {
 #if defined(OS_POSIX)
   int flags = fcntl(fd, F_GETFD);
-  CHECK(flags != -1);
+  CHECK_NE(flags, -1);
   int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
-  CHECK(rc == 0);
+  CHECK_EQ(rc, 0);
 #endif
 }
 #endif
 
+// Represents shared state for all NaClProcessHost objects in the browser.
+// Currently this just handles holding onto the file descriptor for the IRT.
+class NaClBrowser {
+ public:
+  static NaClBrowser* GetInstance() {
+    return Singleton<NaClBrowser>::get();
+  }
+
+  bool IrtAvailable() const {
+    return irt_platform_file_ != base::kInvalidPlatformFileValue;
+  }
+
+  base::PlatformFile IrtFile() const {
+    CHECK_NE(irt_platform_file_, base::kInvalidPlatformFileValue);
+    return irt_platform_file_;
+  }
+
+  // Asynchronously attempt to get the IRT open.
+  bool EnsureIrtAvailable();
+
+  // Make sure the IRT gets opened and follow up with the reply when it's ready.
+  bool MakeIrtAvailable(const base::Closure& reply);
+
+ private:
+  base::PlatformFile irt_platform_file_;
+
+  friend struct DefaultSingletonTraits<NaClBrowser>;
+
+  NaClBrowser()
+      : irt_platform_file_(base::kInvalidPlatformFileValue)
+  {}
+
+  ~NaClBrowser() {
+    if (irt_platform_file_ != base::kInvalidPlatformFileValue)
+      base::ClosePlatformFile(irt_platform_file_);
+  }
+
+  void OpenIrtLibraryFile();
+
+  static void DoOpenIrtLibraryFile() {
+    GetInstance()->OpenIrtLibraryFile();
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(NaClBrowser);
+};
+
 }  // namespace
 
 struct NaClProcessHost::NaClInternal {
@@ -54,17 +100,21 @@ struct NaClProcessHost::NaClInternal {
   std::vector<nacl::Handle> sockets_for_sel_ldr;
 };
 
+static bool RunningOnWOW64() {
+#if defined(OS_WIN)
+  return (base::win::OSInfo::GetInstance()->wow64_status() ==
+          base::win::OSInfo::WOW64_ENABLED);
+#else
+  return false;
+#endif
+}
+
 NaClProcessHost::NaClProcessHost(const std::wstring& url)
     : BrowserChildProcessHost(NACL_LOADER_PROCESS),
       reply_msg_(NULL),
       internal_(new NaClInternal()),
-      running_on_wow64_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   set_name(WideToUTF16Hack(url));
-#if defined(OS_WIN)
-  running_on_wow64_ = (base::win::OSInfo::GetInstance()->wow64_status() ==
-      base::win::OSInfo::WOW64_ENABLED);
-#endif
 }
 
 NaClProcessHost::~NaClProcessHost() {
@@ -92,6 +142,34 @@ NaClProcessHost::~NaClProcessHost() {
   }
 }
 
+// Attempt to ensure the IRT will be available when we need it, but don't wait.
+bool NaClBrowser::EnsureIrtAvailable() {
+  if (IrtAvailable())
+    return true;
+
+  return BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&NaClBrowser::DoOpenIrtLibraryFile));
+}
+
+// We really need the IRT to be available now, so make sure that it is.
+// When it's ready, we'll run the reply closure.
+bool NaClBrowser::MakeIrtAvailable(const base::Closure& reply) {
+  return BrowserThread::PostTaskAndReply(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&NaClBrowser::DoOpenIrtLibraryFile), reply);
+}
+
+// This is called at browser startup.
+// static
+void NaClProcessHost::EarlyStartup() {
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  // Open the IRT file early to make sure that it isn't replaced out from
+  // under us by autoupdate.
+  NaClBrowser::GetInstance()->EnsureIrtAvailable();
+#endif
+}
+
 bool NaClProcessHost::Launch(
     ChromeRenderMessageFilter* chrome_render_message_filter,
     int socket_count,
@@ -107,6 +185,13 @@ bool NaClProcessHost::Launch(
     return false;
   }
 
+  // Start getting the IRT open asynchronously while we launch the NaCl process.
+  // We'll make sure this actually finished in OnProcessLaunched, below.
+  if (!NaClBrowser::GetInstance()->EnsureIrtAvailable()) {
+    LOG(ERROR) << "Cannot launch NaCl process after IRT file open failed";
+    return false;
+  }
+
   // Rather than creating a socket pair in the renderer, and passing
   // one side through the browser to sel_ldr, socket pairs are created
   // in the browser and then passed to the renderer and sel_ldr.
@@ -182,7 +267,7 @@ bool NaClProcessHost::LaunchSelLdr() {
 
   // On Windows we might need to start the broker process to launch a new loader
 #if defined(OS_WIN)
-  if (running_on_wow64_) {
+  if (RunningOnWOW64()) {
     return NaClBrokerService::GetInstance()->LaunchLoader(
         this, ASCIIToWide(channel_id()));
   } else {
@@ -204,7 +289,7 @@ void NaClProcessHost::OnProcessLaunchedByBroker(base::ProcessHandle handle) {
 
 base::TerminationStatus NaClProcessHost::GetChildTerminationStatus(
     int* exit_code) {
-  if (running_on_wow64_)
+  if (RunningOnWOW64())
     return base::GetTerminationStatus(handle(), exit_code);
   return BrowserChildProcessHost::GetChildTerminationStatus(exit_code);
 }
@@ -227,25 +312,15 @@ void NaClProcessHost::OnChildDied() {
   BrowserChildProcessHost::OnChildDied();
 }
 
-FilePath::StringType NaClProcessHost::GetIrtLibraryFilename() {
-  bool on_x86_64 = running_on_wow64_;
-#if defined(__x86_64__)
-  on_x86_64 = true;
-#endif
-  if (on_x86_64) {
-    return FILE_PATH_LITERAL("nacl_irt_x86_64.nexe");
-  } else {
-    return FILE_PATH_LITERAL("nacl_irt_x86_32.nexe");
-  }
-}
+// This only ever runs on the BrowserThread::FILE thread.
+// If multiple tasks are posted, the later ones are no-ops.
+void NaClBrowser::OpenIrtLibraryFile() {
+  if (irt_platform_file_ != base::kInvalidPlatformFileValue)
+    // We've already run.
+    return;
+
+  FilePath irt_filepath;
 
-void NaClProcessHost::OnProcessLaunched() {
-  // TODO(mseaborn): Opening the IRT file every time a NaCl process is
-  // launched probably does not work with auto-update on Linux.  We
-  // might need to open the file on startup.  If so, we would need to
-  // ensure that NaCl's ELF loader does not use lseek() on the shared
-  // IRT file descriptor, otherwise there would be a race condition.
-  FilePath irt_path;
   // Allow the IRT library to be overridden via an environment
   // variable.  This allows the NaCl/Chromium integration bot to
   // specify a newly-built IRT rather than using a prebuilt one
@@ -253,41 +328,103 @@ void NaClProcessHost::OnProcessLaunched() {
   // variable that the standalone NaCl PPAPI plugin accepts.
   const char* irt_path_var = getenv("NACL_IRT_LIBRARY");
   if (irt_path_var != NULL) {
-    FilePath::StringType string(irt_path_var,
-                                irt_path_var + strlen(irt_path_var));
-    irt_path = FilePath(string);
+    FilePath::StringType path_string(
+        irt_path_var, const_cast<const char*>(strchr(irt_path_var, '\0')));
+    irt_filepath = FilePath(path_string);
   } else {
     FilePath plugin_dir;
     if (!PathService::Get(chrome::DIR_INTERNAL_PLUGINS, &plugin_dir)) {
       LOG(ERROR) << "Failed to locate the plugins directory";
-      delete this;
       return;
     }
-    irt_path = plugin_dir.Append(GetIrtLibraryFilename());
+
+    bool on_x86_64 = RunningOnWOW64();
+#if defined(__x86_64__)
+    on_x86_64 = true;
+#endif
+    FilePath::StringType irt_name;
+    if (on_x86_64) {
+      irt_name = FILE_PATH_LITERAL("nacl_irt_x86_64.nexe");
+    } else {
+      irt_name = FILE_PATH_LITERAL("nacl_irt_x86_32.nexe");
+    }
+
+    irt_filepath = plugin_dir.Append(irt_name);
+  }
+
+  base::PlatformFileError error_code;
+  irt_platform_file_ = base::CreatePlatformFile(irt_filepath,
+                                                base::PLATFORM_FILE_OPEN |
+                                                base::PLATFORM_FILE_READ,
+                                                NULL,
+                                                &error_code);
+  if (error_code != base::PLATFORM_FILE_OK) {
+    LOG(ERROR) << "Failed to open NaCl IRT file \""
+               << irt_filepath.LossyDisplayName()
+               << "\": " << error_code;
+  }
+}
+
+void NaClProcessHost::OnProcessLaunched() {
+  NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
+
+  if (nacl_browser->IrtAvailable()) {
+    // The IRT is already open.  Away we go.
+    SendStart(nacl_browser->IrtFile());
+  } else {
+    // We're waiting for the IRT to be open.
+    nacl_browser->MakeIrtAvailable(base::Bind(&NaClProcessHost::IrtReady,
+                                              weak_factory_.GetWeakPtr()));
   }
+}
+
+// The asynchronous attempt to get the IRT file open has completed.
+void NaClProcessHost::IrtReady() {
+  NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
 
-  if (!base::FileUtilProxy::CreateOrOpen(
-           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE),
-           irt_path,
-           base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
-           base::Bind(&NaClProcessHost::OpenIrtFileDone,
-                      weak_factory_.GetWeakPtr()))) {
+  if (nacl_browser->IrtAvailable()) {
+    SendStart(nacl_browser->IrtFile());
+  } else {
+    LOG(ERROR) << "Cannot launch NaCl process after IRT file open failed";
     delete this;
   }
 }
 
-void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
-                                      base::PassPlatformFile file,
-                                      bool created) {
+static bool SendHandleToSelLdr(
+    base::ProcessHandle processh,
+    nacl::Handle sourceh, bool close_source,
+    std::vector<nacl::FileDescriptor> *handles_for_sel_ldr) {
+#if defined(OS_WIN)
+  HANDLE channel;
+  int flags = DUPLICATE_SAME_ACCESS;
+  if (close_source)
+    flags |= DUPLICATE_CLOSE_SOURCE;
+  if (!DuplicateHandle(GetCurrentProcess(),
+                       reinterpret_cast<HANDLE>(sourceh),
+                       processh,
+                       &channel,
+                       0,  // Unused given DUPLICATE_SAME_ACCESS.
+                       FALSE,
+                       flags)) {
+    LOG(ERROR) << "DuplicateHandle() failed";
+    return false;
+  }
+  handles_for_sel_ldr->push_back(
+      reinterpret_cast<nacl::FileDescriptor>(channel));
+#else
+  nacl::FileDescriptor channel;
+  channel.fd = sourceh;
+  channel.auto_close = close_source;
+  handles_for_sel_ldr->push_back(channel);
+#endif
+  return true;
+}
+
+void NaClProcessHost::SendStart(base::PlatformFile irt_file) {
+  CHECK_NE(irt_file, base::kInvalidPlatformFileValue);
+
   std::vector<nacl::FileDescriptor> handles_for_renderer;
   base::ProcessHandle nacl_process_handle;
-  bool have_irt_file = false;
-  if (base::PLATFORM_FILE_OK == error_code) {
-    internal_->sockets_for_sel_ldr.push_back(file.ReleaseValue());
-    have_irt_file = true;
-  } else {
-    LOG(ERROR) << "Failed to open the NaCl IRT library file";
-  }
 
   for (size_t i = 0; i < internal_->sockets_for_renderer.size(); i++) {
 #if defined(OS_WIN)
@@ -347,28 +484,18 @@ void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
 
   std::vector<nacl::FileDescriptor> handles_for_sel_ldr;
   for (size_t i = 0; i < internal_->sockets_for_sel_ldr.size(); i++) {
-#if defined(OS_WIN)
-    HANDLE channel;
-    if (!DuplicateHandle(GetCurrentProcess(),
-                         reinterpret_cast<HANDLE>(
-                             internal_->sockets_for_sel_ldr[i]),
-                         handle(),
-                         &channel,
-                         0,  // Unused given DUPLICATE_SAME_ACCESS.
-                         FALSE,
-                         DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) {
-      LOG(ERROR) << "DuplicateHandle() failed";
+    if (!SendHandleToSelLdr(handle(),
+                            internal_->sockets_for_sel_ldr[i], true,
+                            &handles_for_sel_ldr)) {
       delete this;
       return;
     }
-    handles_for_sel_ldr.push_back(
-        reinterpret_cast<nacl::FileDescriptor>(channel));
-#else
-    nacl::FileDescriptor channel;
-    channel.fd = internal_->sockets_for_sel_ldr[i];
-    channel.auto_close = true;
-    handles_for_sel_ldr.push_back(channel);
-#endif
+  }
+
+  // Send over the IRT file handle.  We don't close our own copy!
+  if (!SendHandleToSelLdr(handle(), irt_file, false, &handles_for_sel_ldr)) {
+    delete this;
+    return;
   }
 
 #if defined(OS_MACOSX)
@@ -393,7 +520,7 @@ void NaClProcessHost::OpenIrtFileDone(base::PlatformFileError error_code,
   handles_for_sel_ldr.push_back(memory_fd);
 #endif
 
-  Send(new NaClProcessMsg_Start(handles_for_sel_ldr, have_irt_file));
+  Send(new NaClProcessMsg_Start(handles_for_sel_ldr));
   internal_->sockets_for_sel_ldr.clear();
 }
 
diff --git a/chrome/browser/nacl_host/nacl_process_host.h b/chrome/browser/nacl_host/nacl_process_host.h
index 59f4f50..ce73491 100644
--- a/chrome/browser/nacl_host/nacl_process_host.h
+++ b/chrome/browser/nacl_host/nacl_process_host.h
@@ -28,6 +28,9 @@ class NaClProcessHost : public BrowserChildProcessHost {
   explicit NaClProcessHost(const std::wstring& url);
   virtual ~NaClProcessHost();
 
+  // Do any minimal work that must be done at browser startup.
+  static void EarlyStartup();
+
   // Initialize the new NaCl process, returning true on success.
   bool Launch(ChromeRenderMessageFilter* chrome_render_message_filter,
               int socket_count,
@@ -50,15 +53,10 @@ class NaClProcessHost : public BrowserChildProcessHost {
 
   bool LaunchSelLdr();
 
-  // Get the architecture-specific filename of NaCl's integrated
-  // runtime (IRT) library, relative to the plugins directory.
-  FilePath::StringType GetIrtLibraryFilename();
-
   virtual void OnProcessLaunched();
 
-  void OpenIrtFileDone(base::PlatformFileError error_code,
-                       base::PassPlatformFile file,
-                       bool created);
+  void IrtReady();
+  void SendStart(base::PlatformFile irt_file);
 
   virtual bool CanShutdown();
 
@@ -73,9 +71,6 @@ class NaClProcessHost : public BrowserChildProcessHost {
   // Socket pairs for the NaCl process and renderer.
   scoped_ptr<NaClInternal> internal_;
 
-  // Windows platform flag
-  bool running_on_wow64_;
-
   base::WeakPtrFactory<NaClProcessHost> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NaClProcessHost);
diff --git a/chrome/browser/renderer_host/chrome_render_message_filter.cc b/chrome/browser/renderer_host/chrome_render_message_filter.cc
index e1c65bd..464f3c7 100644
--- a/chrome/browser/renderer_host/chrome_render_message_filter.cc
+++ b/chrome/browser/renderer_host/chrome_render_message_filter.cc
@@ -147,9 +147,9 @@ void ChromeRenderMessageFilter::OverrideThreadForMessage(
 }
 
 void ChromeRenderMessageFilter::OnLaunchNaCl(
-    const std::wstring& url, int channel_descriptor, IPC::Message* reply_msg) {
+    const std::wstring& url, int socket_count, IPC::Message* reply_msg) {
   NaClProcessHost* host = new NaClProcessHost(url);
-  host->Launch(this, channel_descriptor, reply_msg);
+  host->Launch(this, socket_count, reply_msg);
 }
 
 void ChromeRenderMessageFilter::OnDnsPrefetch(
diff --git a/chrome/browser/renderer_host/chrome_render_message_filter.h b/chrome/browser/renderer_host/chrome_render_message_filter.h
index 9a077df..d35a0ee 100644
--- a/chrome/browser/renderer_host/chrome_render_message_filter.h
+++ b/chrome/browser/renderer_host/chrome_render_message_filter.h
@@ -47,7 +47,7 @@ class ChromeRenderMessageFilter : public BrowserMessageFilter {
   virtual ~ChromeRenderMessageFilter();
 
   void OnLaunchNaCl(const std::wstring& url,
-                    int channel_descriptor,
+                    int socket_count,
                     IPC::Message* reply_msg);
   void OnDnsPrefetch(const std::vector<std::string>& hostnames);
   void OnRendererHistograms(int sequence_number,
diff --git a/chrome/common/nacl_messages.h b/chrome/common/nacl_messages.h
index 5f4aaa6..9297580 100644
--- a/chrome/common/nacl_messages.h
+++ b/chrome/common/nacl_messages.h
@@ -20,9 +20,8 @@
 // NaClProcess messages
 // These are messages sent from the browser to the NaCl process.
 // Tells the NaCl process to start.
-IPC_MESSAGE_CONTROL2(NaClProcessMsg_Start,
-                     std::vector<nacl::FileDescriptor> /* sockets */,
-                     bool /* have_irt_file */)
+IPC_MESSAGE_CONTROL1(NaClProcessMsg_Start,
+                     std::vector<nacl::FileDescriptor> /* sockets */)
 
 // Tells the NaCl broker to launch a NaCl loader process.
 IPC_MESSAGE_CONTROL1(NaClProcessMsg_LaunchLoaderThroughBroker,
@@ -36,4 +35,3 @@ IPC_MESSAGE_CONTROL2(NaClProcessMsg_LoaderLaunched,
 // Notify the broker that all loader processes have been terminated and it
 // should shutdown.
 IPC_MESSAGE_CONTROL0(NaClProcessMsg_StopBroker)
-
diff --git a/chrome/nacl/nacl_listener.cc b/chrome/nacl/nacl_listener.cc
index 678a87f..55f7e43 100644
--- a/chrome/nacl/nacl_listener.cc
+++ b/chrome/nacl/nacl_listener.cc
@@ -96,9 +96,7 @@ bool NaClListener::OnMessageReceived(const IPC::Message& msg) {
   return handled;
 }
 
-void NaClListener::OnStartSelLdr(
-    std::vector<nacl::FileDescriptor> handles,
-    bool have_irt_file) {
+void NaClListener::OnStartSelLdr(std::vector<nacl::FileDescriptor> handles) {
 #if defined(OS_LINUX)
   nacl::SetCreateMemoryObjectFunc(content::MakeSharedMemorySegmentViaIPC);
 #elif defined(OS_MACOSX)
@@ -108,22 +106,22 @@ void NaClListener::OnStartSelLdr(
   handles.pop_back();
 #endif
 
-  if (have_irt_file) {
-    CHECK(handles.size() >= 1);
-    NaClHandle irt_handle = nacl::ToNativeHandle(handles[handles.size() - 1]);
-    handles.pop_back();
+  CHECK(handles.size() >= 1);
+  NaClHandle irt_handle = nacl::ToNativeHandle(handles[handles.size() - 1]);
+  handles.pop_back();
+
 #if defined(OS_WIN)
-    int irt_desc = _open_osfhandle(reinterpret_cast<intptr_t>(irt_handle),
-                                   _O_RDWR | _O_BINARY);
-    if (irt_desc < 0) {
-      LOG(ERROR) << "_open_osfhandle() failed";
-      return;
-    }
+  int irt_desc = _open_osfhandle(reinterpret_cast<intptr_t>(irt_handle),
+                                 _O_RDONLY | _O_BINARY);
+  if (irt_desc < 0) {
+    LOG(ERROR) << "_open_osfhandle() failed";
+    return;
+  }
 #else
-    int irt_desc = irt_handle;
+  int irt_desc = irt_handle;
 #endif
-    NaClSetIrtFileDesc(irt_desc);
-  }
+
+  NaClSetIrtFileDesc(irt_desc);
 
   scoped_array<NaClHandle> array(new NaClHandle[handles.size()]);
   for (size_t i = 0; i < handles.size(); i++) {
diff --git a/chrome/nacl/nacl_listener.h b/chrome/nacl/nacl_listener.h
index f0d275e..9a81b65 100644
--- a/chrome/nacl/nacl_listener.h
+++ b/chrome/nacl/nacl_listener.h
@@ -22,8 +22,7 @@ class NaClListener : public IPC::Channel::Listener {
   void set_debug_enabled(bool value) {debug_enabled_ = value;}
 
  private:
-  void OnStartSelLdr(std::vector<nacl::FileDescriptor> handles,
-                     bool have_irt_file);
+  void OnStartSelLdr(std::vector<nacl::FileDescriptor> handles);
   virtual bool OnMessageReceived(const IPC::Message& msg);
 
   bool debug_enabled_;