Bug 86854 – [Qt] REGRESSION(r117501): It made almost all tests assert in debug mode

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Csaba Osztrogonác]

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]

[Viatcheslav Ostapenko]

Created attachment 142734 [details]
Patch

[Brady Eidson]

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.

[Rafael Brandao]

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.

[Andreas Kling]

Comment on attachment 142734 [details]
Patch

Looks great to me. Thanks for the fix!

[Viatcheslav Ostapenko]

(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.

[Viatcheslav Ostapenko]

Created attachment 142751 [details]
Patch for commit.

[WebKit Review Bot]

Comment on attachment 142751 [details]
Patch for commit.

Clearing flags on attachment: 142751

Committed r117625: <http://trac.webkit.org/changeset/117625>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.