Changeset 30811 in webkit for trunk/JavaScriptCore/wtf/MessageQueue.h

Timestamp:

Mar 5, 2008, 2:26:34 PM (17 years ago)

Author:

[email protected]

Message:

JavaScriptCore:

Reviewed by Alexey and Mark Rowe

Fix for <rdar://problem/5778247> - Reproducible crash on storage/execute-sql-args.html

DatabaseThread::unscheduleDatabaseTasks() manually filters through a MessageQueue,
removing particular items for Databases that were shutting down.

This filtering operation is not atomic, and therefore causes a race condition with the
MessageQueue waking up and reading from the message queue.

The end result was an attempt to dereference a null DatabaseTask. Timing-wise, this never
seemed to happen in a debug build, otherwise an assertion would've caught it. Replacing that
assertion with a crash in a release build is what revealed this bug.

• wtf/MessageQueue.h: (WTF::::waitForMessage): Tweak the waiting logic to check the queue's empty state then go back to sleep if the queue was empty - checking m_killed each time it wakes up.

WebCore:

Reviewed by Alexey and Mark Rowe

Fix for <rdar://problem/5778247> - Reproducible crash on storage/execute-sql-args.html

DatabaseThread::unscheduleDatabaseTasks() manually filters through a MessageQueue,
removing particular items for Databases that were shutting down.

This filtering operation is not atomic, and therefore causes a race condition with the
database thread waking up and reading from the message queue.

The end result was an attempt to dereference a null DatabaseTask. Timing-wise, this never
seemed to happen in a debug build, otherwise an assertion would've caught it. Replacing that
assertion with a crash in a release build is what revealed this bug.

The fix for the above symptom was entirely in WTF::MessageQueue in JSCore. With this fix in
place, another crash popped up in the layout tests that was related to dereferencing a
deallocated object - simply because SQLTransaction had a raw pointer to it's Database object
when it needed to be a ref pointer.

• storage/SQLTransaction.cpp: (WebCore::SQLTransaction::runCurrentStatement):
• storage/SQLTransaction.h: Change m_database to be a RefPtr (WebCore::SQLTransaction::database):

LayoutTests:

Reviewed by Alexey + Mark Rowe

Fix for <rdar://problem/5778247> - Reproducible crash on storage/execute-sql-args.html

This test takes its best shot at handling two databases on a single database thread at once,
then having one of those databases go away completely (garbage collection and everything)

• storage/multiple-databases-garbage-collection-expected.txt: Added.
• storage/multiple-databases-garbage-collection.html: Added.

File:

• trunk/JavaScriptCore/wtf/MessageQueue.h (modified) (1 diff)

trunk/JavaScriptCore/wtf/MessageQueue.h

@@ -76 +76 @@
     {
         MutexLocker lock(m_mutex);
-        if (m_killed)
-            return false;
         
-        if (m_queue.isEmpty())
+        while (!m_killed && m_queue.isEmpty())
             m_condition.wait(m_mutex);
+
         if (m_killed)
             return false;