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) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

[Wez, 2017/01/14 01:53:11]

bradnelson: Turns out that this breaks several tests, which call SetDelegate()
before initializing any BrowserThreads. Ideally I'd prefer that we require that
this be called before BrowserThreads exist even in Chrome itself, so that there
is no risk of racing with other threads - see my comment in Chrome MainParts.

+  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_CURRENTLY_ON(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();
 }
 
 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
 
 NaClBrowser* NaClBrowser::GetInstance() {
-  return base::Singleton<NaClBrowser>::get();
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  static NaClBrowser* g_instance = nullptr;
+  if (!g_instance)
+    g_instance = new NaClBrowser();
+  return g_instance;
 }
 
 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(
+void NaClBrowser::SetGdbDebugStubPortListenerForTest(
     base::Callback<void(int)> listener) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   debug_stub_port_listener_ = listener;
 }
 
-void NaClBrowser::ClearGdbDebugStubPortListener() {
+void NaClBrowser::ClearGdbDebugStubPortListenerForTest() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   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