Summary: | [Qt] REGRESSION(r117501): It made almost all tests assert in debug mode | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||
Component: | Tools / Tests | Assignee: | 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
Csaba Osztrogonác
2012-05-18 07:24:20 PDT
Created attachment 142734 [details]
Patch
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 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 on attachment 142734 [details]
Patch
Looks great to me. Thanks for the fix!
(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. Created attachment 142751 [details]
Patch for commit.
Comment on attachment 142751 [details] Patch for commit. Clearing flags on attachment: 142751 Committed r117625: <http://trac.webkit.org/changeset/117625> All reviewed patches have been landed. Closing bug. |