From 2a1272586d0ab6719820ea25ac596683b33d0676 Mon Sep 17 00:00:00 2001
From: "darin@google.com"
<darin@google.com@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue, 5 Aug 2008 23:16:41 +0000
Subject: ObjectWatcher needs to know when the current thread's MessageLoop is
being destroyed. This might also be generically useful, so I added a new API
on ML to observe when the ML is being destroyed. The notification is sent to
observers just prior to ML::current() being modified to return NULL.
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@407 0039d316-1c4b-4281-b951-d872f2087c98
---
base/message_loop.cc | 18 ++++++++++++++++++
base/message_loop.h | 29 ++++++++++++++++++++++++++---
base/object_watcher.cc | 13 ++++++++++++-
base/object_watcher.h | 7 +++++--
base/object_watcher_unittest.cc | 33 +++++++++++++++++++++++++++++++++
5 files changed, 94 insertions(+), 6 deletions(-)
diff --git a/base/message_loop.cc b/base/message_loop.cc
index 7d2fbff..78f820c 100644
--- a/base/message_loop.cc
+++ b/base/message_loop.cc
@@ -140,9 +140,17 @@ MessageLoop::MessageLoop() : message_hwnd_(NULL),
MessageLoop::~MessageLoop() {
DCHECK(this == current());
+
+ // Let interested parties have one last shot at accessing this.
+ FOR_EACH_OBSERVER(DestructionObserver, destruction_observers_,
+ WillDestroyCurrentMessageLoop());
+
+ // OK, now make it so that no one can find us.
ThreadLocalStorage::Set(tls_index_, NULL);
+
DCHECK(!dispatcher_);
DCHECK(!quit_received_ && !quit_now_);
+
// Most tasks that have not been Run() are deleted in the |timer_manager_|
// destructor after we remove our tls index. We delete the tasks in our
// queues here so their destuction is similar to the tasks in the
@@ -158,6 +166,16 @@ void MessageLoop::SetThreadName(const std::string& thread_name) {
StartHistogrammer();
}
+void MessageLoop::AddDestructionObserver(DestructionObserver *obs) {
+ DCHECK(this == current());
+ destruction_observers_.AddObserver(obs);
+}
+
+void MessageLoop::RemoveDestructionObserver(DestructionObserver *obs) {
+ DCHECK(this == current());
+ destruction_observers_.RemoveObserver(obs);
+}
+
void MessageLoop::AddObserver(Observer *obs) {
DCHECK(this == current());
observers_.AddObserver(obs);
diff --git a/base/message_loop.h b/base/message_loop.h
index 1610d07..cd7b1c5 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -150,6 +150,10 @@ class MessageLoop {
virtual void OnObjectSignaled(HANDLE object) = 0;
};
+ // Have the current thread's message loop watch for a signaled object.
+ // Pass a null watcher to stop watching the object.
+ bool WatchObject(HANDLE, Watcher*);
+
// Dispatcher is used during a nested invocation of Run to dispatch events.
// If Run is invoked with a non-NULL Dispatcher, MessageLoop does not
// dispatch events (or invoke TranslateMessage), rather every message is
@@ -172,9 +176,27 @@ class MessageLoop {
virtual bool Dispatch(const MSG& msg) = 0;
};
- // Have the current thread's message loop watch for a signaled object.
- // Pass a null watcher to stop watching the object.
- bool WatchObject(HANDLE, Watcher*);
+ // A DestructionObserver is notified when the current MessageLoop is being
+ // destroyed. These obsevers are notified prior to MessageLoop::current()
+ // being changed to return NULL. This gives interested parties the chance to
+ // do final cleanup that depends on the MessageLoop.
+ //
+ // NOTE: Any tasks posted to the MessageLoop during this notification will
+ // not be run. Instead, they will be deleted.
+ //
+ class DestructionObserver {
+ public:
+ virtual ~DestructionObserver() {}
+ virtual void WillDestroyCurrentMessageLoop() = 0;
+ };
+
+ // Add a DestructionObserver, which will start receiving notifications
+ // immediately.
+ void AddDestructionObserver(DestructionObserver* destruction_observer);
+
+ // Remove a DestructionObserver. It is safe to call this method while a
+ // DestructionObserver is receiving a notification callback.
+ void RemoveDestructionObserver(DestructionObserver* destruction_observer);
// An Observer is an object that receives global notifications from the
// MessageLoop.
@@ -577,6 +599,7 @@ class MessageLoop {
std::vector<Watcher*> watchers_;
ObserverList<Observer> observers_;
+ ObserverList<DestructionObserver> destruction_observers_;
HWND message_hwnd_;
IDMap<Task> timed_tasks_;
// A recursion block that prevents accidentally running additonal tasks when
diff --git a/base/object_watcher.cc b/base/object_watcher.cc
index 5e8c27b..9872a4c 100644
--- a/base/object_watcher.cc
+++ b/base/object_watcher.cc
@@ -29,7 +29,6 @@
#include "base/object_watcher.h"
-#include "base/message_loop.h"
#include "base/logging.h"
namespace base {
@@ -91,6 +90,10 @@ bool ObjectWatcher::StartWatching(HANDLE object, Delegate* delegate) {
}
watch_ = watch;
+
+ // We need to know if the current message loop is going away so we can
+ // prevent the wait thread from trying to access a dead message loop.
+ MessageLoop::current()->AddDestructionObserver(this);
return true;
}
@@ -124,6 +127,8 @@ bool ObjectWatcher::StopWatching() {
delete watch_;
watch_ = NULL;
+
+ MessageLoop::current()->RemoveDestructionObserver(this);
return true;
}
@@ -142,4 +147,10 @@ void CALLBACK ObjectWatcher::DoneWaiting(void* param, BOOLEAN timed_out) {
watch->origin_loop->PostTask(FROM_HERE, watch);
}
+void ObjectWatcher::WillDestroyCurrentMessageLoop() {
+ // Need to shutdown the watch so that we don't try to access the MessageLoop
+ // after this point.
+ StopWatching();
+}
+
} // namespace base
diff --git a/base/object_watcher.h b/base/object_watcher.h
index de4816a..9058ab4 100644
--- a/base/object_watcher.h
+++ b/base/object_watcher.h
@@ -32,7 +32,7 @@
#include <windows.h>
-#include "base/basictypes.h"
+#include "base/message_loop.h"
namespace base {
@@ -64,7 +64,7 @@ namespace base {
// scope, the watcher_ will be destroyed, and there is no need to worry about
// OnObjectSignaled being called on a deleted MyClass pointer. Easy!
//
-class ObjectWatcher {
+class ObjectWatcher : public MessageLoop::DestructionObserver {
public:
class Delegate {
public:
@@ -97,6 +97,9 @@ class ObjectWatcher {
// Called on a background thread when done waiting.
static void CALLBACK DoneWaiting(void* param, BOOLEAN timed_out);
+ // MessageLoop::DestructionObserver implementation:
+ virtual void WillDestroyCurrentMessageLoop();
+
// Internal state.
struct Watch;
Watch* watch_;
diff --git a/base/object_watcher_unittest.cc b/base/object_watcher_unittest.cc
index 51ed968..6c3fcce 100644
--- a/base/object_watcher_unittest.cc
+++ b/base/object_watcher_unittest.cc
@@ -27,6 +27,8 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#include <process.h>
+
#include "base/message_loop.h"
#include "base/object_watcher.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -113,3 +115,34 @@ TEST(ObjectWatcherTest, CancelAfterSet) {
CloseHandle(event);
}
+
+// Used so we can simulate a MessageLoop that dies before an ObjectWatcher.
+// This ordinarily doesn't happen when people use the Thread class, but it can
+// happen when people use the Singleton pattern or atexit.
+static unsigned __stdcall ThreadFunc(void* param) {
+ HANDLE event = CreateEvent(NULL, TRUE, FALSE, NULL); // not signaled
+ {
+ base::ObjectWatcher watcher;
+ {
+ MessageLoop message_loop;
+
+ QuitDelegate delegate;
+ watcher.StartWatching(event, &delegate);
+ }
+ }
+ CloseHandle(event);
+ return 0;
+}
+
+TEST(ObjectWatcherTest, OutlivesMessageLoop) {
+ unsigned int thread_id;
+ HANDLE thread = reinterpret_cast<HANDLE>(
+ _beginthreadex(NULL,
+ 0,
+ ThreadFunc,
+ NULL,
+ 0,
+ &thread_id));
+ WaitForSingleObject(thread, INFINITE);
+ CloseHandle(thread);
+}
--
cgit v1.1