summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorwillchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-10 22:24:53 +0000
committerwillchan@chromium.org <willchan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-10 22:24:53 +0000
commita40e548394014c8679c269fb936a57e68e632080 (patch)
tree9ecab63f54ba8ce7be569da87b2d364f48dca391
parentaebe23a3cd89181c929618093ec8a0fefb4b4f11 (diff)
downloadchromium_src-a40e548394014c8679c269fb936a57e68e632080.zip
chromium_src-a40e548394014c8679c269fb936a57e68e632080.tar.gz
chromium_src-a40e548394014c8679c269fb936a57e68e632080.tar.bz2
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=none Review URL: http://codereview.chromium.org/5693003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68893 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--base/platform_thread_posix.cc4
-rw-r--r--base/platform_thread_win.cc4
-rw-r--r--chrome/browser/browser_process_impl.cc7
-rw-r--r--chrome/browser/geolocation/geolocation_provider.cc5
-rw-r--r--chrome/browser/printing/printer_query.cc5
5 files changed, 25 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..68e837e 100644
--- a/base/platform_thread_win.cc
+++ b/base/platform_thread_win.cc
@@ -125,6 +125,10 @@ bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) {
// static
void PlatformThread::Join(PlatformThreadHandle thread_handle) {
DCHECK(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();
// 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 ea52797..8facb15 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();
}