diff options
author | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-14 00:40:00 +0000 |
---|---|---|
committer | willchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-14 00:40:00 +0000 |
commit | 7d038c3ed504066d9e39e3306281bb5dd047cfd2 (patch) | |
tree | df768ce2f474305f0e6c7f99eb9a0458b8cd46bd | |
parent | aa64c82121827c0943380f9c2c1063c1337af018 (diff) | |
download | chromium_src-7d038c3ed504066d9e39e3306281bb5dd047cfd2.zip chromium_src-7d038c3ed504066d9e39e3306281bb5dd047cfd2.tar.gz chromium_src-7d038c3ed504066d9e39e3306281bb5dd047cfd2.tar.bz2 |
Reland r68893 after fixing allowing blocking IO on shutdown.
This time I just turned off the checking for Chrome/Windows because Chrome/Windows shutdown is a clusterf***. TODO(willchan): Give a shit about Windows, get a machine that runs Windows, debug Chrome shutdown on Windows, enable this ThreadRestrictions for Chrome/Windows.
Consider PlatformThread::Join() to be blocking IO.
Marks PlatformThread::Join() as blocking IO using ThreadRestrictions.
Whitelists existing spots where we join on the UI/IO threads.
Also noteworthy is I allow blocking IO on shutdown.
BUG=65530, 66077, 66082
TEST=existing
Review URL: http://codereview.chromium.org/5750003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@69079 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | base/platform_thread_posix.cc | 4 | ||||
-rw-r--r-- | base/platform_thread_win.cc | 8 | ||||
-rw-r--r-- | chrome/browser/browser_process_impl.cc | 7 | ||||
-rw-r--r-- | chrome/browser/geolocation/geolocation_provider.cc | 5 | ||||
-rw-r--r-- | chrome/browser/printing/printer_query.cc | 5 |
5 files changed, 29 insertions, 0 deletions
diff --git a/base/platform_thread_posix.cc b/base/platform_thread_posix.cc index e442e9c..9807ac6 100644 --- a/base/platform_thread_posix.cc +++ b/base/platform_thread_posix.cc @@ -219,5 +219,9 @@ bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) { // static void PlatformThread::Join(PlatformThreadHandle thread_handle) { + // Joining another thread may block the current thread for a long time, since + // the thread referred to by |thread_handle| may still be running long-lived / + // blocking tasks. + base::ThreadRestrictions::AssertIOAllowed(); pthread_join(thread_handle, NULL); } diff --git a/base/platform_thread_win.cc b/base/platform_thread_win.cc index e5afc52..ac8a5db 100644 --- a/base/platform_thread_win.cc +++ b/base/platform_thread_win.cc @@ -125,6 +125,14 @@ bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) { // static void PlatformThread::Join(PlatformThreadHandle thread_handle) { DCHECK(thread_handle); + // TODO(willchan): Enable this check once I can get it to work for Windows + // shutdown. + // Joining another thread may block the current thread for a long time, since + // the thread referred to by |thread_handle| may still be running long-lived / + // blocking tasks. +#if 0 + base::ThreadRestrictions::AssertIOAllowed(); +#endif // Wait for the thread to exit. It should already have terminated but make // sure this assumption is valid. diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index 53d44d8..756aa6c 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -13,6 +13,7 @@ #include "base/path_service.h" #include "base/task.h" #include "base/thread.h" +#include "base/thread_restrictions.h" #include "base/waitable_event.h" #include "chrome/browser/appcache/chrome_appcache_service.h" #include "chrome/browser/automation/automation_provider_list.h" @@ -252,6 +253,12 @@ unsigned int BrowserProcessImpl::ReleaseModule() { DCHECK_NE(0u, module_ref_count_); module_ref_count_--; if (0 == module_ref_count_) { + // Allow UI and IO threads to do blocking IO on shutdown, since we do a lot + // of it on shutdown for valid reasons. + base::ThreadRestrictions::SetIOAllowed(true); + io_thread()->message_loop()->PostTask( + FROM_HERE, + NewRunnableFunction(&base::ThreadRestrictions::SetIOAllowed, true)); MessageLoop::current()->PostTask( FROM_HERE, NewRunnableFunction(DidEndMainMessageLoop)); MessageLoop::current()->Quit(); diff --git a/chrome/browser/geolocation/geolocation_provider.cc b/chrome/browser/geolocation/geolocation_provider.cc index 97e9049..3756af7 100644 --- a/chrome/browser/geolocation/geolocation_provider.cc +++ b/chrome/browser/geolocation/geolocation_provider.cc @@ -5,6 +5,7 @@ #include "chrome/browser/geolocation/geolocation_provider.h" #include "base/singleton.h" +#include "base/thread_restrictions.h" #include "chrome/browser/geolocation/location_arbitrator.h" // This class is guaranteed to outlive its internal thread, so ref counting @@ -47,6 +48,10 @@ bool GeolocationProvider::RemoveObserver(GeolocationObserver* observer) { void GeolocationProvider::OnObserversChanged() { DCHECK(OnClientThread()); if (observers_.empty()) { + // http://crbug.com/66077: This is a bug. The geolocation thread may + // transitively (via other threads it joins) block on long-running tasks / + // IO. + base::ThreadRestrictions::ScopedAllowIO allow_io; Stop(); } else { if (!IsRunning()) { diff --git a/chrome/browser/printing/printer_query.cc b/chrome/browser/printing/printer_query.cc index c28f400..7b73d86 100644 --- a/chrome/browser/printing/printer_query.cc +++ b/chrome/browser/printing/printer_query.cc @@ -5,6 +5,7 @@ #include "chrome/browser/printing/printer_query.h" #include "base/message_loop.h" +#include "base/thread_restrictions.h" #include "chrome/browser/printing/print_job_worker.h" namespace printing { @@ -96,6 +97,10 @@ void PrinterQuery::GetSettings(GetSettingsAskParam ask_user_for_settings, void PrinterQuery::StopWorker() { if (worker_.get()) { + // http://crbug.com/66082: We're blocking on the PrinterQuery's worker + // thread. It's not clear to me if this may result in blocking the current + // thread for an unacceptable time. We should probably fix it. + base::ThreadRestrictions::ScopedAllowIO allow_io; worker_->Stop(); worker_.reset(); } |