Bug 42826 – Crash in thread pool because WorkQueue keeps waiting on closed HANDLEs

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Adam Roben (:aroben)]

There's no way to tell WorkQueue to stop waiting on a particular HANDLE. This means that users of WorkQueue either have to leak their HANDLEs (yuck) or close their HANDLEs while WorkQueue is still waiting on them, which leads to crashes in WaitForMultipleObjects (also yuck).

We should add a way to tell WorkQueue to stop waiting on a particular handle so that it can be closed.

[Adam Roben (:aroben)]

It's possible that this is a cause of bug 42785. See discussion in that bug for more details.

[Adam Roben (:aroben)]

<rdar://problem/8222253>

[Adam Roben (:aroben)]

See bug 42785 comments 1 through 8.

[Adam Roben (:aroben)]

(In reply to bug 42785 comment #8)
> I think using a pair of events will work. The first event will be used to tell the work queue thread to stop waiting. The second event will be used to tell the main thread that waiting has stopped.

Anders informed me that this is not a good strategy. Waiting on the work queue thread is dangerous because the work queue thread might wait on the first thread, resulting in a deadlock.

I think adding an unregisterAndCloseHandle function to WorkQueue might be the way to go. We'd add the handle to a m_handlesToClose Vector and signal m_performWorkEvent. Then the work queue thread would close all handles in m_handlesToClose when it wakes up.

[Adam Roben (:aroben)]

Fixing this doesn't seem to fix bug 42785.

[Adam Roben (:aroben)]

*** Bug 43148 has been marked as a duplicate of this bug. ***

[Adam Roben (:aroben)]

I've been working on switching over to using an I/O completion port in WorkQueue. This would make it unnecessary to unregister a HANDLE; you can just close it and stop worrying.

However, I'm seeing spurious reads in the web process. I.e., the web process will request 1 byte of the next message from the pipe, and then will be told that 207 bytes were read. This doesn't make any sense! Windows shouldn't be reading more than we asked it to; where would it store the data?

[Adam Roben (:aroben)]

I'm going to base the fix for this on the fix for bug 43150.

[Adam Roben (:aroben)]

Created attachment 65752 [details]
Teach WorkQueue how to stop waiting on objects on Windows

[Adam Roben (:aroben)]

Comment on attachment 65752 [details]
Teach WorkQueue how to stop waiting on objects on Windows

This patch introduces a crash due to WorkItemWin being destroyed when it is currently being used in handleCallback.

To fix this we shouldn't destroy the WorkItemWin until we've unregistered the wait.

[Adam Roben (:aroben)]

Created attachment 66933 [details]
Teach WorkQueue how to stop waiting on objects on Windows

[Adam Roben (:aroben)]

Committed r67013: <http://trac.webkit.org/changeset/67013>

[WebKit Review Bot]

http://trac.webkit.org/changeset/67013 might have broken Qt Linux Release
The following changes are on the blame list:
http://trac.webkit.org/changeset/67013
http://trac.webkit.org/changeset/67014