Bug 68783 – IconDatabase’s use of ThreadCondition leads to assertion failures in the face of spurious wakeups

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mark Rowe (bdash)]

The code in IconDatabase::syncThreadMainLoop assumes that if the call to ThreadCondition::wait returns then it must have been due to the call to ThreadCondition::signal.  However, spurious wakeups (<http://en.wikipedia.org/wiki/Spurious_wakeup>) can lead to wait returning without the condition being signaled. When this happens we’ll hit the following assertion in a debug build:

            ASSERT(m_disabledSuddenTerminationForSyncThread);

In a release build we’ll get all sorts of console spew due to the resulting unbalanced call to enableSuddenTermination.

<rdar://problem/10177824>

[Mark Rowe (bdash)]

Created attachment 108616 [details]
Patch v1

[WebKit Review Bot]

Attachment 108616 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Rowe (bdash)]

This patch isn’t quite right. If wakeSyncThread is called on the main thread while the sync thread is executing the body of the sync thread loop (e.g., when the lock has been dropped) then m_disabledSuddenTerminationForSyncThread and m_syncThreadHasWorkToDo will be set, but the block of code immediately before we call ThreadCondition::wait will reset m_disabledSuddenTerminationForSyncThread to false, causing us to then fail the assertion immediately after the call to ThreadCondition::wait.

[Mark Rowe (bdash)]

Created attachment 108618 [details]
Updated patch

[WebKit Review Bot]

Attachment 108618 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Rowe (bdash)]

Landed in r95929.

[Adam Roben (:aroben)]

This is causing assertion failures for me on launch:

ASSERTION FAILED: m_disabledSuddenTerminationForSyncThread
/Users/aroben/dev/WebKit/OpenSource/Source/WebCore/loader/icon/IconDatabase.cpp(1441) : void *WebCore::IconDatabase::syncThreadMainLoop()
1   WebCore::IconDatabase::syncThreadMainLoop()
2   WebCore::IconDatabase::iconDatabaseSyncThread()
3   WebCore::IconDatabase::iconDatabaseSyncThreadStart(void*)
4   _ZN3WTFL16threadEntryPointEPv
5   _pthread_start
6   thread_start

It looks like wakeSyncThread is being called before the sync thread has even started. This causes shouldReenableSuddenTermination to be set to true and m_disabledSuddenTerminationForSyncThread to be set to false at the start of syncThreadMainLoop.

[Mark Rowe (bdash)]

Filed bug 68809 to follow up on that.