Bug 86854

Summary: [Qt] REGRESSION(r117501): It made almost all tests assert in debug mode
Product: WebKit Reporter: Csaba Osztrogon√°c <ossy>
Component: Tools / TestsAssignee: Viatcheslav Ostapenko <ostap73>
Status: RESOLVED FIXED    
Severity: Blocker CC: beidson, japhet, kling, ostap73, webkit.review.bot
Priority: P1 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 79668, 85799    
Attachments:
Description Flags
Patch
kling: review+, kling: commit-queue-
Patch for commit. none

Description Csaba Osztrogon√°c 2012-05-18 07:24:20 PDT
examples:
Qt 64 bit debug bot: http://build.webkit.sed.hu/results/x86-64%20Linux%20Qt%20Debug/r117580%20%2822936%29/results.html
Qt 32 bit debug bot: http://build.webkit.sed.hu/results/x86-32%20Linux%20Qt%20Debug/r117587%20%2820625%29/results.html

crash log for DumpRenderTree (pid 8877):
STDOUT: <empty>
STDERR: ASSERTION FAILED: m_retainOrReleaseIconRequested
STDERR: ../../../../Source/WebCore/loader/icon/IconDatabase.cpp(1510) : void WebCore::IconDatabase::performPendingRetainAndReleaseOperations()
STDERR: 1   0xf51388f6 /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::IconDatabase::performPendingRetainAndReleaseOperations()+0x56) [0xf51388f6]
STDERR: 2   0xf5137f25 /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::IconDatabase::performURLImport()+0x6f9) [0xf5137f25]
STDERR: 3   0xf51366af /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::IconDatabase::iconDatabaseSyncThread()+0x4cf) [0xf51366af]
STDERR: 4   0xf51361d9 /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(WebCore::IconDatabase::iconDatabaseSyncThreadStart(void*)+0x23) [0xf51361d9]
STDERR: 5   0xf5c54cc2 /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(+0x3279cc2) [0xf5c54cc2]
STDERR: 6   0xf5c6b910 /mnt/raptor3/slaves/qt-linux-32-debug/build/WebKitBuild/Debug/lib/libQtWebKit.so.4(+0x3290910) [0xf5c6b910]
STDERR: 7   0xf0e957b0 /lib/libpthread.so.0(+0x57b0) [0xf0e957b0]
STDERR: 8   0xf0bb10be /lib/libc.so.6(clone+0x5e) [0xf0bb10be]
Comment 1 Viatcheslav Ostapenko 2012-05-18 10:32:12 PDT
Created attachment 142734 [details]
Patch
Comment 2 Brady Eidson 2012-05-18 10:36:20 PDT
Comment on attachment 142734 [details]
Patch

Seems okay to me.  kling should really take a look, as he is the "most recently intimate" with the lock intricacies involved.
Comment 3 Rafael Brandao 2012-05-18 10:47:39 PDT
Comment on attachment 142734 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142734&action=review

Did you try using tryLock?

> Source/WebCore/loader/icon/IconDatabase.cpp:1423
> +            // Previous flag check was done outside of the lock and flag could be changed by antoher thread.

Typo on another.
Comment 4 Andreas Kling 2012-05-18 11:16:24 PDT
Comment on attachment 142734 [details]
Patch

Looks great to me. Thanks for the fix!
Comment 5 Viatcheslav Ostapenko 2012-05-18 11:37:58 PDT
(In reply to comment #3)
> (From update of attachment 142734 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142734&action=review
> 
> Did you try using tryLock?

Why tryLock?
I wouldn't touch mutex in the thread loop until it is really necessary. Double checking of the flag, IMHO, quite common.

> > Source/WebCore/loader/icon/IconDatabase.cpp:1423
> > +            // Previous flag check was done outside of the lock and flag could be changed by antoher thread.
> 
> Typo on another.

Thanks.
Comment 6 Viatcheslav Ostapenko 2012-05-18 11:45:00 PDT
Created attachment 142751 [details]
Patch for commit.
Comment 7 WebKit Review Bot 2012-05-18 14:09:50 PDT
Comment on attachment 142751 [details]
Patch for commit.

Clearing flags on attachment: 142751

Committed r117625: <http://trac.webkit.org/changeset/117625>
Comment 8 WebKit Review Bot 2012-05-18 14:09:56 PDT
All reviewed patches have been landed.  Closing bug.