summaryrefslogtreecommitdiffstats
path: root/base/message_loop.cc
diff options
context:
space:
mode:
authorsky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-09-15 22:14:36 +0000
committersky@chromium.org <sky@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-09-15 22:14:36 +0000
commit23c386ba6127f23296da9998be78805da7c769fa (patch)
tree4496b80c46d42b9c656e97370c394bae2ba13946 /base/message_loop.cc
parent51043776a6749c69d59feb34199bd2d9fe9cb045 (diff)
downloadchromium_src-23c386ba6127f23296da9998be78805da7c769fa.zip
chromium_src-23c386ba6127f23296da9998be78805da7c769fa.tar.gz
chromium_src-23c386ba6127f23296da9998be78805da7c769fa.tar.bz2
Fixes bad URLRequest leak detection. nss_ocsp creates a
URLRequest. This URLRequest, if valid, is deleted when the IO thread is being shutdown by way of a DestructionObserver attached to the IO thread. Prior to this patch this proves problematic as the LeakTracker is run before DestructionObservers, which means we were errorenously detecting a leak and causing random UI failures (and perhaps other tests). To fix this I've moved the LeakTracker to run after the IO thread MessageLoop has been deleted (which means DestructionObservers have been notified). Doing this triggered a DCHECK in ChromeNetLog::AddEntry: DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); Here's the full stack: StackTrace::StackTrace() [0xf247c1] logging::LogMessage::~LogMessage() [0xf3ef4f] ChromeNetLog::AddEntry() [0x5c9639] net::BoundNetLog::AddEntry() [0x147d8a3] net::BoundNetLog::AddEvent() [0x147d92a] net::internal::ClientSocketPoolBaseHelper::CancelRequest() [0x132d032] net::ClientSocketPoolBase<>::CancelRequest() [0x134aabd] net::TCPClientSocketPool::CancelRequest() [0x1348fdb] net::ClientSocketHandle::ResetInternal() [0x13273b7] net::ClientSocketHandle::Reset() [0x1327874] net::ClientSocketHandle::~ClientSocketHandle() [0x1327895] scoped_ptr<>::~scoped_ptr() [0x1304b97] net::HttpStreamRequest::~HttpStreamRequest() [0x1300933] base::RefCounted<>::Release() [0xa15b83] scoped_refptr<>::operator=() [0x12f2da4] net::HttpNetworkTransaction::~HttpNetworkTransaction() [0x12ee377] scoped_ptr<>::~scoped_ptr() [0x12e2724] net::HttpCache::Transaction::~Transaction() [0x12e10a6] scoped_ptr<>::reset() [0x12d1f0b] URLRequestHttpJob::DestroyTransaction() [0x142229e] URLRequestHttpJob::Kill() [0x14252e5] URLRequest::DoCancel() [0x137af1e] URLRequest::Cancel() [0x137b081] (anonymous namespace)::OCSPRequestSession::CancelURLRequest() [0x13142a7] (anonymous namespace)::OCSPRequestSession::WillDestroyCurrentMessageLoop() [0x1314b30] MessageLoop::~MessageLoop() [0xf421ab] base::Thread::ThreadMain() [0xf71395] ThreadFunc() [0xf54338] start_thread [0x7ff66c6d83f7] 0x7ff669d5cbbd This is still on the IO thread, but the DCHECK fails because at the time the DestructionObservers are run Thread::message_loop() returns NULL, which means the DCHECK fails. I've fixed this by changing the DCHECK to pass if the current thread's message loop is NULL. I feel a bit quesy about this as it seems a bit fragile (well, all this code is fragile). I would be inclined to make Thread::message_loop() return the MessageLoop until after the destructor has run, but this seems equally risky. Let me know what you prefer. BUG=52022 TEST=none Review URL: http://codereview.chromium.org/3402006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@59566 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'base/message_loop.cc')
-rw-r--r--base/message_loop.cc18
1 files changed, 10 insertions, 8 deletions
diff --git a/base/message_loop.cc b/base/message_loop.cc
index 32ac12c..56d1418 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -181,24 +181,26 @@ MessageLoop::~MessageLoop() {
lazy_tls_ptr.Pointer()->Set(NULL);
}
-void MessageLoop::AddDestructionObserver(DestructionObserver *obs) {
+void MessageLoop::AddDestructionObserver(
+ DestructionObserver* destruction_observer) {
DCHECK(this == current());
- destruction_observers_.AddObserver(obs);
+ destruction_observers_.AddObserver(destruction_observer);
}
-void MessageLoop::RemoveDestructionObserver(DestructionObserver *obs) {
+void MessageLoop::RemoveDestructionObserver(
+ DestructionObserver* destruction_observer) {
DCHECK(this == current());
- destruction_observers_.RemoveObserver(obs);
+ destruction_observers_.RemoveObserver(destruction_observer);
}
-void MessageLoop::AddTaskObserver(TaskObserver *obs) {
+void MessageLoop::AddTaskObserver(TaskObserver* task_observer) {
DCHECK_EQ(this, current());
- task_observers_.AddObserver(obs);
+ task_observers_.AddObserver(task_observer);
}
-void MessageLoop::RemoveTaskObserver(TaskObserver *obs) {
+void MessageLoop::RemoveTaskObserver(TaskObserver* task_observer) {
DCHECK_EQ(this, current());
- task_observers_.RemoveObserver(obs);
+ task_observers_.RemoveObserver(task_observer);
}
void MessageLoop::Run() {