[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
aroben at apple.com
aroben at apple.com
Wed Dec 22 13:12:00 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit a1e2fea26ff23173e5a08454e5e70f844fdb29fa
Author: aroben at apple.com <aroben at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Sep 8 21:16:45 2010 +0000
Teach WorkQueue how to stop waiting on objects on Windows
WorkQueue now uses a subclass of WorkItemWin, HandleWorkItem, to hold
the waited-upon HANDLE and its corresponding wait handle. When a
HANDLE is unregistered, we use the HandleWorkItem to cancel the wait
and destroy the HANDLE.
Fixes <http://webkit.org/b/42826> <rdar://problem/8222253> Crash in
thread pool because WorkQueue keeps waiting on closed HANDLEs
Reviewed by Anders Carlsson.
* Platform/CoreIPC/win/ConnectionWin.cpp:
(CoreIPC::Connection::platformInvalidate): Changed to call
WorkQueue::unregisterAndCloseHandle instead of closing our handles
manually.
(CoreIPC::Connection::readEventHandler):
(CoreIPC::Connection::writeEventHandler):
Handle cases where the pipe has already closed by just bailing out.
This can happen if a WorkItem to call one of these functions has
already been scheduled before platformInvalidate is called.
* Platform/WorkQueue.h: Gave WorkItemWin a virtual destructor, added
HandleWorkItem, changed m_handles to hold HandleWorkItems, and added
functions for unregistering waits.
* Platform/win/WorkQueueWin.cpp:
(WorkQueue::WorkItemWin::~WorkItemWin): Added. This virtual destructor
ensures that HandleWorkItem's destructor gets called.
(WorkQueue::HandleWorkItem::HandleWorkItem):
(WorkQueue::HandleWorkItem::createByAdoptingHandle):
Added simple constructor/creator.
(WorkQueue::HandleWorkItem::~HandleWorkItem): Closes the handle we
adopted.
(WorkQueue::registerHandle): Changed to create a HandleWorkItemWin and
to store the wait handle in it.
(WorkQueue::unregisterAndCloseHandle): Added. Removes the
HandleWorkItem for this HANDLE from m_handles and then schedules its
wait to be unregistered and the item to be destroyed.
(WorkQueue::platformInvalidate): Added an assertion and removed an
obsolete FIXME.
(WorkQueue::unregisterWaitAndDestroyItemSoon): Added. Calls
unregisterWaitAndDestroyItemCallback on a worker thread, passing it
ownership of the HandleWorkItem.
(WorkQueue::unregisterWaitAndDestroyItemCallback): Added. Adopts the
passed-in HandleWorkItem, then unregisters the handle's wait, then
destroys the HandleWorkItem when the RefPtr holding it goes out of
scope. Destroying the HandleWorkItem closes the handle.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@67013 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebKit2/ChangeLog b/WebKit2/ChangeLog
index 479eccb..d13edb5 100644
--- a/WebKit2/ChangeLog
+++ b/WebKit2/ChangeLog
@@ -1,5 +1,59 @@
2010-09-08 Adam Roben <aroben at apple.com>
+ Teach WorkQueue how to stop waiting on objects on Windows
+
+ WorkQueue now uses a subclass of WorkItemWin, HandleWorkItem, to hold
+ the waited-upon HANDLE and its corresponding wait handle. When a
+ HANDLE is unregistered, we use the HandleWorkItem to cancel the wait
+ and destroy the HANDLE.
+
+ Fixes <http://webkit.org/b/42826> <rdar://problem/8222253> Crash in
+ thread pool because WorkQueue keeps waiting on closed HANDLEs
+
+ Reviewed by Anders Carlsson.
+
+ * Platform/CoreIPC/win/ConnectionWin.cpp:
+ (CoreIPC::Connection::platformInvalidate): Changed to call
+ WorkQueue::unregisterAndCloseHandle instead of closing our handles
+ manually.
+
+ (CoreIPC::Connection::readEventHandler):
+ (CoreIPC::Connection::writeEventHandler):
+ Handle cases where the pipe has already closed by just bailing out.
+ This can happen if a WorkItem to call one of these functions has
+ already been scheduled before platformInvalidate is called.
+
+ * Platform/WorkQueue.h: Gave WorkItemWin a virtual destructor, added
+ HandleWorkItem, changed m_handles to hold HandleWorkItems, and added
+ functions for unregistering waits.
+
+ * Platform/win/WorkQueueWin.cpp:
+ (WorkQueue::WorkItemWin::~WorkItemWin): Added. This virtual destructor
+ ensures that HandleWorkItem's destructor gets called.
+
+ (WorkQueue::HandleWorkItem::HandleWorkItem):
+ (WorkQueue::HandleWorkItem::createByAdoptingHandle):
+ Added simple constructor/creator.
+
+ (WorkQueue::HandleWorkItem::~HandleWorkItem): Closes the handle we
+ adopted.
+ (WorkQueue::registerHandle): Changed to create a HandleWorkItemWin and
+ to store the wait handle in it.
+ (WorkQueue::unregisterAndCloseHandle): Added. Removes the
+ HandleWorkItem for this HANDLE from m_handles and then schedules its
+ wait to be unregistered and the item to be destroyed.
+ (WorkQueue::platformInvalidate): Added an assertion and removed an
+ obsolete FIXME.
+ (WorkQueue::unregisterWaitAndDestroyItemSoon): Added. Calls
+ unregisterWaitAndDestroyItemCallback on a worker thread, passing it
+ ownership of the HandleWorkItem.
+ (WorkQueue::unregisterWaitAndDestroyItemCallback): Added. Adopts the
+ passed-in HandleWorkItem, then unregisters the handle's wait, then
+ destroys the HandleWorkItem when the RefPtr holding it goes out of
+ scope. Destroying the HandleWorkItem closes the handle.
+
+2010-09-08 Adam Roben <aroben at apple.com>
+
Remove WKSerializedScriptValue.cpp/h from the Copy Files build phase
* WebKit2.xcodeproj/project.pbxproj:
diff --git a/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp b/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
index f1cc46c..695da78 100644
--- a/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
+++ b/WebKit2/Platform/CoreIPC/win/ConnectionWin.cpp
@@ -98,12 +98,10 @@ void Connection::platformInvalidate()
if (m_connectionPipe == INVALID_HANDLE_VALUE)
return;
- // FIXME: Unregister the handles.
-
- ::CloseHandle(m_readState.hEvent);
+ m_connectionQueue.unregisterAndCloseHandle(m_readState.hEvent);
m_readState.hEvent = 0;
- ::CloseHandle(m_writeState.hEvent);
+ m_connectionQueue.unregisterAndCloseHandle(m_writeState.hEvent);
m_writeState.hEvent = 0;
::CloseHandle(m_connectionPipe);
@@ -112,6 +110,9 @@ void Connection::platformInvalidate()
void Connection::readEventHandler()
{
+ if (m_connectionPipe == INVALID_HANDLE_VALUE)
+ return;
+
while (true) {
// Check if we got some data.
DWORD numberOfBytesRead = 0;
@@ -231,6 +232,9 @@ void Connection::readEventHandler()
void Connection::writeEventHandler()
{
+ if (m_connectionPipe == INVALID_HANDLE_VALUE)
+ return;
+
DWORD numberOfBytesWritten = 0;
if (!::GetOverlappedResult(m_connectionPipe, &m_writeState, &numberOfBytesWritten, FALSE)) {
DWORD error = ::GetLastError();
diff --git a/WebKit2/Platform/WorkQueue.h b/WebKit2/Platform/WorkQueue.h
index 364eed7..83132a5 100644
--- a/WebKit2/Platform/WorkQueue.h
+++ b/WebKit2/Platform/WorkQueue.h
@@ -68,7 +68,7 @@ public:
void unregisterMachPortEventHandler(mach_port_t);
#elif PLATFORM(WIN)
void registerHandle(HANDLE, PassOwnPtr<WorkItem>);
- void unregisterHandle(HANDLE);
+ void unregisterAndCloseHandle(HANDLE);
#elif PLATFORM(QT)
void connectSignal(QObject*, const char* signal, PassOwnPtr<WorkItem>);
void disconnectSignal(QObject*, const char* signal);
@@ -96,17 +96,34 @@ private:
class WorkItemWin : public RefCounted<WorkItemWin> {
public:
static PassRefPtr<WorkItemWin> create(PassOwnPtr<WorkItem>, WorkQueue*);
+ virtual ~WorkItemWin();
WorkItem* item() const { return m_item.get(); }
WorkQueue* queue() const { return m_queue; }
- private:
+ protected:
WorkItemWin(PassOwnPtr<WorkItem>, WorkQueue*);
+ private:
OwnPtr<WorkItem> m_item;
WorkQueue* m_queue;
};
+ class HandleWorkItem : public WorkItemWin {
+ public:
+ static PassRefPtr<HandleWorkItem> createByAdoptingHandle(HANDLE, PassOwnPtr<WorkItem>, WorkQueue*);
+ virtual ~HandleWorkItem();
+
+ void setWaitHandle(HANDLE waitHandle) { m_waitHandle = waitHandle; }
+ HANDLE waitHandle() const { return m_waitHandle; }
+
+ private:
+ HandleWorkItem(HANDLE, PassOwnPtr<WorkItem>, WorkQueue*);
+
+ HANDLE m_handle;
+ HANDLE m_waitHandle;
+ };
+
static void CALLBACK handleCallback(void* context, BOOLEAN timerOrWaitFired);
static DWORD WINAPI workThreadCallback(void* context);
@@ -114,13 +131,16 @@ private:
void unregisterAsWorkThread();
void performWorkOnRegisteredWorkThread();
+ static void unregisterWaitAndDestroyItemSoon(PassRefPtr<HandleWorkItem>);
+ static DWORD WINAPI unregisterWaitAndDestroyItemCallback(void* context);
+
volatile LONG m_isWorkThreadRegistered;
Mutex m_workItemQueueLock;
Vector<RefPtr<WorkItemWin> > m_workItemQueue;
Mutex m_handlesLock;
- HashMap<HANDLE, RefPtr<WorkItemWin> > m_handles;
+ HashMap<HANDLE, RefPtr<HandleWorkItem> > m_handles;
#elif PLATFORM(QT)
class WorkItemQt;
HashMap<QObject*, WorkItemQt*> m_signalListeners;
diff --git a/WebKit2/Platform/win/WorkQueueWin.cpp b/WebKit2/Platform/win/WorkQueueWin.cpp
index 9fbe641..98e2802 100644
--- a/WebKit2/Platform/win/WorkQueueWin.cpp
+++ b/WebKit2/Platform/win/WorkQueueWin.cpp
@@ -38,6 +38,28 @@ PassRefPtr<WorkQueue::WorkItemWin> WorkQueue::WorkItemWin::create(PassOwnPtr<Wor
return adoptRef(new WorkItemWin(item, queue));
}
+WorkQueue::WorkItemWin::~WorkItemWin()
+{
+}
+
+inline WorkQueue::HandleWorkItem::HandleWorkItem(HANDLE handle, PassOwnPtr<WorkItem> item, WorkQueue* queue)
+ : WorkItemWin(item, queue)
+ , m_handle(handle)
+ , m_waitHandle(0)
+{
+ ASSERT_ARG(handle, handle);
+}
+
+PassRefPtr<WorkQueue::HandleWorkItem> WorkQueue::HandleWorkItem::createByAdoptingHandle(HANDLE handle, PassOwnPtr<WorkItem> item, WorkQueue* queue)
+{
+ return adoptRef(new HandleWorkItem(handle, item, queue));
+}
+
+WorkQueue::HandleWorkItem::~HandleWorkItem()
+{
+ ::CloseHandle(m_handle);
+}
+
void WorkQueue::handleCallback(void* context, BOOLEAN timerOrWaitFired)
{
ASSERT_ARG(context, context);
@@ -65,20 +87,32 @@ void WorkQueue::handleCallback(void* context, BOOLEAN timerOrWaitFired)
void WorkQueue::registerHandle(HANDLE handle, PassOwnPtr<WorkItem> item)
{
- RefPtr<WorkItemWin> itemWin = WorkItemWin::create(item, this);
+ RefPtr<HandleWorkItem> handleItem = HandleWorkItem::createByAdoptingHandle(handle, item, this);
- // Add the item.
{
- MutexLocker locker(m_handlesLock);
- m_handles.set(handle, itemWin);
+ MutexLocker lock(m_handlesLock);
+ ASSERT_ARG(handle, !m_handles.contains(handle));
+ m_handles.set(handle, handleItem);
}
- // FIXME: We need to hold onto waitHandle so that we can unregister the wait later.
HANDLE waitHandle;
- if (!::RegisterWaitForSingleObject(&waitHandle, handle, handleCallback, itemWin.get(), INFINITE, WT_EXECUTEDEFAULT)) {
+ if (!::RegisterWaitForSingleObject(&waitHandle, handle, handleCallback, handleItem.get(), INFINITE, WT_EXECUTEDEFAULT)) {
DWORD error = ::GetLastError();
ASSERT_NOT_REACHED();
}
+ handleItem->setWaitHandle(waitHandle);
+}
+
+void WorkQueue::unregisterAndCloseHandle(HANDLE handle)
+{
+ RefPtr<HandleWorkItem> item;
+ {
+ MutexLocker locker(m_handlesLock);
+ ASSERT_ARG(handle, m_handles.contains(handle));
+ item = m_handles.take(handle);
+ }
+
+ unregisterWaitAndDestroyItemSoon(item.release());
}
DWORD WorkQueue::workThreadCallback(void* context)
@@ -146,7 +180,10 @@ void WorkQueue::unregisterAsWorkThread()
void WorkQueue::platformInvalidate()
{
- // FIXME: Stop the thread and do other cleanup.
+#if !ASSERT_DISABLED
+ MutexLocker lock(m_handlesLock);
+ ASSERT(m_handles.isEmpty());
+#endif
}
void WorkQueue::scheduleWork(PassOwnPtr<WorkItem> item)
@@ -164,3 +201,28 @@ void WorkQueue::scheduleWork(PassOwnPtr<WorkItem> item)
if (!m_isWorkThreadRegistered)
::QueueUserWorkItem(workThreadCallback, this, WT_EXECUTEDEFAULT);
}
+
+void WorkQueue::unregisterWaitAndDestroyItemSoon(PassRefPtr<HandleWorkItem> item)
+{
+ // We're going to make a blocking call to ::UnregisterWaitEx before closing the handle. (The
+ // blocking version of ::UnregisterWaitEx is much simpler than the non-blocking version.) If we
+ // do this on the current thread, we'll deadlock if we're currently in a callback function for
+ // the wait we're unregistering. So instead we do it asynchronously on some other worker thread.
+
+ ::QueueUserWorkItem(unregisterWaitAndDestroyItemCallback, item.leakRef(), WT_EXECUTEDEFAULT);
+}
+
+DWORD WINAPI WorkQueue::unregisterWaitAndDestroyItemCallback(void* context)
+{
+ ASSERT_ARG(context, context);
+ RefPtr<HandleWorkItem> item = adoptRef(static_cast<HandleWorkItem*>(context));
+
+ // Now that we know we're not in a callback function for the wait we're unregistering, we can
+ // make a blocking call to ::UnregisterWaitEx.
+ if (!::UnregisterWaitEx(item->waitHandle(), INVALID_HANDLE_VALUE)) {
+ DWORD error = ::GetLastError();
+ ASSERT_NOT_REACHED();
+ }
+
+ return 0;
+}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list