Add thread checks to NaClBrowser, and make it leaky

components/nacl/browser/nacl_browser.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/nacl/browser/nacl_browser.h"
 
 #include <stddef.h>
 
 #include "base/command_line.h"
 #include "base/files/file_proxy.h"
 #include "base/files/file_util.h"
+#include "base/lazy_instance.h"
 #include "base/location.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/path_service.h"
 #include "base/pickle.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 89 matching lines @@
 void LogCacheSet(nacl::NaClBrowser::ValidationCacheStatus status) {
   // Bucket zero is reserved for future use.
   UMA_HISTOGRAM_ENUMERATION("NaCl.ValidationCache.Set", status,
                             nacl::NaClBrowser::CACHE_MAX);
 }
 
 // Crash throttling parameters.
 const size_t kMaxCrashesPerInterval = 3;
 const int64_t kCrashesIntervalInSeconds = 120;
 
+// Holds the NaClBrowserDelegate, which is leaked on shutdown.
+NaClBrowserDelegate* g_browser_delegate = nullptr;
+
 }  // namespace
 
 namespace nacl {
 
 base::File OpenNaClReadExecImpl(const base::FilePath& file_path,
                                 bool is_executable) {
   // Get a file descriptor. On Windows, we need 'GENERIC_EXECUTE' in order to
   // memory map the executable.
   // IMPORTANT: This file descriptor must not have write access - that could
   // allow a NaCl inner sandbox escape.
@@ skipping 15 matching lines @@
 }
 
 NaClBrowser::NaClBrowser()
     : irt_filepath_(),
       irt_state_(NaClResourceUninitialized),
       validation_cache_file_path_(),
       validation_cache_is_enabled_(false),
       validation_cache_is_modified_(false),
       validation_cache_state_(NaClResourceUninitialized),
       path_cache_(kFilePathCacheSize),
-      ok_(true),
-      weak_factory_(this) {
+      has_failed_(false) {
 #if !defined(OS_ANDROID)
       validation_cache_is_enabled_ =
           CheckEnvVar("NACL_VALIDATION_CACHE",
                       kValidationCacheEnabledByDefault);
 #endif
+      DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 }
 
-void NaClBrowser::SetDelegate(NaClBrowserDelegate* delegate) {
-  NaClBrowser* nacl_browser = NaClBrowser::GetInstance();
-  nacl_browser->browser_delegate_.reset(delegate);
+void NaClBrowser::SetDelegate(std::unique_ptr<NaClBrowserDelegate> delegate) {
+  // In the browser SetDelegate is called after threads are initialized.
+  // In tests it is called before initializing BrowserThreads.
+  if (content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI)) {
+    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  }
+  DCHECK(delegate);
+  DCHECK(!g_browser_delegate);
+  g_browser_delegate = delegate.release();
 }
 
 NaClBrowserDelegate* NaClBrowser::GetDelegate() {
-  // The delegate is not owned by the IO thread.  This accessor method can be
-  // called from other threads.
-  DCHECK(GetInstance()->browser_delegate_.get() != NULL);
-  return GetInstance()->browser_delegate_.get();
+  // NaClBrowser calls this on the IO thread, not the UI thread.
+  DCHECK(g_browser_delegate);
+  return g_browser_delegate;
+}
+
+void NaClBrowser::ClearAndDeleteDelegateForTest() {
+  DCHECK(
+      !content::BrowserThread::IsThreadInitialized(content::BrowserThread::UI));
+  DCHECK(g_browser_delegate);
+  delete g_browser_delegate;
+  g_browser_delegate = nullptr;
 }
 
 void NaClBrowser::EarlyStartup() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   InitIrtFilePath();
   InitValidationCacheFilePath();
 }
 
 NaClBrowser::~NaClBrowser() {
+  NOTREACHED();

[bradnelson, 2017/02/23 03:07:43]

Good idea

 }
 
 void NaClBrowser::InitIrtFilePath() {
   // 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
   // downloaded via Chromium's DEPS file.  We use the same environment
   // variable that the standalone NaCl PPAPI plugin accepts.
   const char* irt_path_var = getenv("NACL_IRT_LIBRARY");
   if (irt_path_var != NULL) {
     base::FilePath::StringType path_string(
         irt_path_var, const_cast<const char*>(strchr(irt_path_var, '\0')));
     irt_filepath_ = base::FilePath(path_string);
   } else {
     base::FilePath plugin_dir;
-    if (!browser_delegate_->GetPluginDirectory(&plugin_dir)) {
+    if (!GetDelegate()->GetPluginDirectory(&plugin_dir)) {
       DLOG(ERROR) << "Failed to locate the plugins directory, NaCl disabled.";
       MarkAsFailed();
       return;
     }
     irt_filepath_ = plugin_dir.Append(NaClIrtName());
   }
 }
 
 #if defined(OS_WIN)
 bool NaClBrowser::GetNaCl64ExePath(base::FilePath* exe_path) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   base::FilePath module_path;
   if (!PathService::Get(base::FILE_MODULE, &module_path)) {
     LOG(ERROR) << "NaCl process launch failed: could not resolve module";
     return false;
   }
   *exe_path = module_path.DirName().Append(L"nacl64");
   return true;
 }
 #endif
 
+// static
+NaClBrowser* NaClBrowser::GetInstanceInternal() {
+  static NaClBrowser* g_instance = nullptr;
+  if (!g_instance)
+    g_instance = new NaClBrowser();
+  return g_instance;
+}
+
 NaClBrowser* NaClBrowser::GetInstance() {
-  return base::Singleton<NaClBrowser>::get();
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  return GetInstanceInternal();
 }
 
 bool NaClBrowser::IsReady() const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   return (IsOk() &&
           irt_state_ == NaClResourceReady &&
           validation_cache_state_ == NaClResourceReady);
 }
 
 bool NaClBrowser::IsOk() const {
-  return ok_;
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  return !has_failed_;
 }
 
 const base::File& NaClBrowser::IrtFile() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   CHECK_EQ(irt_state_, NaClResourceReady);
   CHECK(irt_file_.IsValid());
   return irt_file_;
 }
 
 void NaClBrowser::EnsureAllResourcesAvailable() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   EnsureIrtAvailable();
   EnsureValidationCacheAvailable();
 }
 
 // Load the IRT async.
 void NaClBrowser::EnsureIrtAvailable() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (IsOk() && irt_state_ == NaClResourceUninitialized) {
     irt_state_ = NaClResourceRequested;
     // TODO(ncbray) use blocking pool.
     std::unique_ptr<base::FileProxy> file_proxy(
         new base::FileProxy(content::BrowserThread::GetTaskRunnerForThread(
                                 content::BrowserThread::FILE)
                                 .get()));
     base::FileProxy* proxy = file_proxy.get();
-    if (!proxy->CreateOrOpen(irt_filepath_,
-                             base::File::FLAG_OPEN | base::File::FLAG_READ,
-                             base::Bind(&NaClBrowser::OnIrtOpened,
-                                        weak_factory_.GetWeakPtr(),
-                                        Passed(&file_proxy)))) {
+    if (!proxy->CreateOrOpen(
+            irt_filepath_, base::File::FLAG_OPEN | base::File::FLAG_READ,
+            base::Bind(&NaClBrowser::OnIrtOpened, base::Unretained(this),
+                       base::Passed(&file_proxy)))) {
       LOG(ERROR) << "Internal error, NaCl disabled.";
       MarkAsFailed();
     }
   }
 }
 
 void NaClBrowser::OnIrtOpened(std::unique_ptr<base::FileProxy> file_proxy,
                               base::File::Error error_code) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK_EQ(irt_state_, NaClResourceRequested);
   if (file_proxy->IsValid()) {
     irt_file_ = file_proxy->TakeFile();
   } else {
     LOG(ERROR) << "Failed to open NaCl IRT file \""
                << irt_filepath_.LossyDisplayName()
                << "\": " << error_code;
     MarkAsFailed();
   }
   irt_state_ = NaClResourceReady;
   CheckWaiting();
 }
 
 void NaClBrowser::SetProcessGdbDebugStubPort(int process_id, int port) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   gdb_debug_stub_port_map_[process_id] = port;
   if (port != kGdbDebugStubPortUnknown &&
       !debug_stub_port_listener_.is_null()) {
     content::BrowserThread::PostTask(
         content::BrowserThread::IO,
         FROM_HERE,
         base::Bind(debug_stub_port_listener_, port));
   }
 }
 
-void NaClBrowser::SetGdbDebugStubPortListener(
+// static
+void NaClBrowser::SetGdbDebugStubPortListenerForTest(
     base::Callback<void(int)> listener) {
-  debug_stub_port_listener_ = listener;
+  GetInstanceInternal()->debug_stub_port_listener_ = listener;
 }
 
-void NaClBrowser::ClearGdbDebugStubPortListener() {
-  debug_stub_port_listener_.Reset();
+// static
+void NaClBrowser::ClearGdbDebugStubPortListenerForTest() {
+  GetInstanceInternal()->debug_stub_port_listener_.Reset();
 }
 
 int NaClBrowser::GetProcessGdbDebugStubPort(int process_id) {
+  // Called from TaskManager TaskGroup impl, on CrBrowserMain.
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   GdbDebugStubPortMap::iterator i = gdb_debug_stub_port_map_.find(process_id);
   if (i != gdb_debug_stub_port_map_.end()) {
     return i->second;
   }
   return kGdbDebugStubPortUnused;
 }
 
 void NaClBrowser::InitValidationCacheFilePath() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // Determine where the validation cache resides in the file system.  It
   // exists in Chrome's cache directory and is not tied to any specific
   // profile.
   // Start by finding the user data directory.
   base::FilePath user_data_dir;
-  if (!browser_delegate_->GetUserDirectory(&user_data_dir)) {
+  if (!GetDelegate()->GetUserDirectory(&user_data_dir)) {
     RunWithoutValidationCache();
     return;
   }
   // The cache directory may or may not be the user data directory.
   base::FilePath cache_file_path;
-  browser_delegate_->GetCacheDirectory(&cache_file_path);
+  GetDelegate()->GetCacheDirectory(&cache_file_path);
   // Append the base file name to the cache directory.
 
   validation_cache_file_path_ =
       cache_file_path.Append(kValidationCacheFileName);
 }
 
 void NaClBrowser::EnsureValidationCacheAvailable() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (IsOk() && validation_cache_state_ == NaClResourceUninitialized) {
     if (ValidationCacheIsEnabled()) {
       validation_cache_state_ = NaClResourceRequested;
 
       // Structure for carrying data between the callbacks.
       std::string* data = new std::string();
       // We can get away not giving this a sequence ID because this is the first
       // task and further file access will not occur until after we get a
       // response.
       if (!content::BrowserThread::PostBlockingPoolTaskAndReply(
               FROM_HERE,
               base::Bind(ReadCache, validation_cache_file_path_, data),
               base::Bind(&NaClBrowser::OnValidationCacheLoaded,
-                         weak_factory_.GetWeakPtr(),
-                         base::Owned(data)))) {
+                         base::Unretained(this), base::Owned(data)))) {
         RunWithoutValidationCache();
       }
     } else {
       RunWithoutValidationCache();
     }
   }
 }
 
 void NaClBrowser::OnValidationCacheLoaded(const std::string *data) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // Did the cache get cleared before the load completed?  If so, ignore the
   // incoming data.
   if (validation_cache_state_ == NaClResourceReady)
     return;
 
   if (data->size() == 0) {
     // No file found.
     validation_cache_.Reset();
   } else {
     base::Pickle pickle(data->data(), data->size());
     validation_cache_.Deserialize(&pickle);
   }
   validation_cache_state_ = NaClResourceReady;
   CheckWaiting();
 }
 
 void NaClBrowser::RunWithoutValidationCache() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // Be paranoid.
   validation_cache_.Reset();
   validation_cache_is_enabled_ = false;
   validation_cache_state_ = NaClResourceReady;
   CheckWaiting();
 }
 
 void NaClBrowser::CheckWaiting() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (!IsOk() || IsReady()) {
     // Queue the waiting tasks into the message loop.  This helps avoid
     // re-entrancy problems that could occur if the closure was invoked
     // directly.  For example, this could result in use-after-free of the
     // process host.
     for (std::vector<base::Closure>::iterator iter = waiting_.begin();
          iter != waiting_.end(); ++iter) {
       base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, *iter);
     }
     waiting_.clear();
   }
 }
 
 void NaClBrowser::MarkAsFailed() {
-  ok_ = false;
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  has_failed_ = true;
   CheckWaiting();
 }
 
 void NaClBrowser::WaitForResources(const base::Closure& reply) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   waiting_.push_back(reply);
   EnsureAllResourcesAvailable();
   CheckWaiting();
 }
 
 const base::FilePath& NaClBrowser::GetIrtFilePath() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   return irt_filepath_;
 }
 
 void NaClBrowser::PutFilePath(const base::FilePath& path,
                               uint64_t* file_token_lo,
                               uint64_t* file_token_hi) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   while (true) {
     uint64_t file_token[2] = {base::RandUint64(), base::RandUint64()};
     // A zero file_token indicates there is no file_token, if we get zero, ask
@@ skipping 92 matching lines @@
 
   // If the cache is cleared before it is loaded from the filesystem, act as if
   // we just loaded an empty cache.
   if (validation_cache_state_ != NaClResourceReady) {
     validation_cache_state_ = NaClResourceReady;
     CheckWaiting();
   }
 }
 
 void NaClBrowser::MarkValidationCacheAsModified() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (!validation_cache_is_modified_) {
     // Wait before persisting to disk.  This can coalesce multiple cache
     // modifications info a single disk write.
     base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
         FROM_HERE, base::Bind(&NaClBrowser::PersistValidationCache,
-                              weak_factory_.GetWeakPtr()),
+                              base::Unretained(this)),
         base::TimeDelta::FromMilliseconds(kValidationCacheCoalescingTimeMS));
     validation_cache_is_modified_ = true;
   }
 }
 
 void NaClBrowser::PersistValidationCache() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // validation_cache_is_modified_ may be false if the cache was cleared while
   // this delayed task was pending.
   // validation_cache_file_path_ may be empty if something went wrong during
   // initialization.
   if (validation_cache_is_modified_ && !validation_cache_file_path_.empty()) {
     base::Pickle* pickle = new base::Pickle();
     validation_cache_.Serialize(pickle);
 
     // Pass the serialized data to another thread to write to disk.  File IO is
     // not allowed on the IO thread (which is the thread this method runs on)
     // because it can degrade the responsiveness of the browser.
     // The task is sequenced so that multiple writes happen in order.
     content::BrowserThread::PostBlockingPoolSequencedTask(
         kValidationCacheSequenceName,
         FROM_HERE,
         base::Bind(WriteCache, validation_cache_file_path_,
                    base::Owned(pickle)));
   }
   validation_cache_is_modified_ = false;
 }
 
 void NaClBrowser::OnProcessEnd(int process_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   gdb_debug_stub_port_map_.erase(process_id);
 }
 
 void NaClBrowser::OnProcessCrashed() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (crash_times_.size() == kMaxCrashesPerInterval) {
     crash_times_.pop_front();
   }
   base::Time time = base::Time::Now();
   crash_times_.push_back(time);
 }
 
 bool NaClBrowser::IsThrottled() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (crash_times_.size() != kMaxCrashesPerInterval) {
     return false;
   }
   base::TimeDelta delta = base::Time::Now() - crash_times_.front();
   return delta.InSeconds() <= kCrashesIntervalInSeconds;
 }
 
 }  // namespace nacl