Changeset 117744 in webkit
- Timestamp:
- May 21, 2012 1:14:46 AM (12 years ago)
- Location:
- trunk/Source
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WTF/ChangeLog
r117478 r117744 1 2012-05-21 Andreas Kling <kling@webkit.org> 2 3 REGRESSION(r117501): IconDatabase asserts on startup in synchronousIconForPageURL(). 4 <http://webkit.org/b/86935> 5 <rdar://problem/11480012> 6 7 Reviewed by Anders Carlsson. 8 9 Added a swap() to HashCountedSet. 10 11 * wtf/HashCountedSet.h: 12 (HashCountedSet::swap): 13 1 14 2012-05-16 Geoffrey Garen <ggaren@apple.com> 2 15 -
trunk/Source/WTF/wtf/HashCountedSet.h
r112555 r117744 40 40 41 41 HashCountedSet() {} 42 43 void swap(HashCountedSet&); 42 44 43 45 int size() const; … … 76 78 ImplType m_impl; 77 79 }; 80 81 template<typename Value, typename HashFunctions, typename Traits> 82 inline void HashCountedSet<Value, HashFunctions, Traits>::swap(HashCountedSet& other) 83 { 84 m_impl.swap(other.m_impl); 85 } 78 86 79 87 template<typename Value, typename HashFunctions, typename Traits> -
trunk/Source/WebCore/ChangeLog
r117743 r117744 1 2012-05-18 Andreas Kling <kling@webkit.org> 2 3 REGRESSION(r117501): IconDatabase asserts on startup in synchronousIconForPageURL(). 4 <http://webkit.org/b/86935> 5 <rdar://problem/11480012> 6 7 Reviewed by Anders Carlsson. 8 9 - Correctly set m_retainOrReleaseIconRequested to true in retainIconForPageURL(). 10 This was causing the assertions, as we would end up doing nothing until the first 11 icon release request came in. 12 13 - Require that m_urlsToRetainOrReleaseLock be held when accessing m_retainOrReleaseIconRequested. 14 This removes a possible race condition in double checked locking. 15 16 - Swap over the retain/release work queues while holding m_urlsToRetainOrReleaseLock 17 and release it right away to avoid sitting on the lock while updating the database. 18 19 * loader/icon/IconDatabase.cpp: 20 (WebCore::IconDatabase::synchronousIconForPageURL): 21 (WebCore::IconDatabase::retainIconForPageURL): 22 (WebCore::IconDatabase::releaseIconForPageURL): 23 (WebCore::IconDatabase::retainedPageURLCount): 24 (WebCore::IconDatabase::performURLImport): 25 (WebCore::IconDatabase::syncThreadMainLoop): 26 (WebCore::IconDatabase::performPendingRetainAndReleaseOperations): 27 * loader/icon/IconDatabase.h: 28 (IconDatabase): 29 1 30 2012-05-21 Kent Tamura <tkent@chromium.org> 2 31 -
trunk/Source/WebCore/loader/icon/IconDatabase.cpp
r117625 r117744 224 224 MutexLocker locker(m_urlAndIconLock); 225 225 226 if (m_retainOrReleaseIconRequested) 227 performPendingRetainAndReleaseOperations(); 226 performPendingRetainAndReleaseOperations(); 228 227 229 228 String pageURLCopy; // Creates a null string for easy testing … … 399 398 if (!isEnabled() || !documentCanHaveIcon(pageURL)) 400 399 return; 401 402 MutexLocker locker(m_urlsToRetainOrReleaseLock); 403 m_urlsToRetain.add(pageURL); 400 401 { 402 MutexLocker locker(m_urlsToRetainOrReleaseLock); 403 m_urlsToRetain.add(pageURL); 404 m_retainOrReleaseIconRequested = true; 405 } 406 404 407 scheduleOrDeferSyncTimer(); 405 408 } … … 449 452 return; 450 453 451 MutexLocker locker(m_urlsToRetainOrReleaseLock); 452 m_urlsToRelease.add(pageURL); 453 m_retainOrReleaseIconRequested = true; 454 { 455 MutexLocker locker(m_urlsToRetainOrReleaseLock); 456 m_urlsToRelease.add(pageURL); 457 m_retainOrReleaseIconRequested = true; 458 } 454 459 scheduleOrDeferSyncTimer(); 455 460 } … … 745 750 { 746 751 MutexLocker locker(m_urlAndIconLock); 747 748 if (m_retainOrReleaseIconRequested) 749 performPendingRetainAndReleaseOperations(); 750 752 performPendingRetainAndReleaseOperations(); 751 753 return m_retainedPageURLs.size(); 752 754 } … … 1335 1337 MutexLocker locker(m_urlAndIconLock); 1336 1338 1337 if (m_retainOrReleaseIconRequested) 1338 performPendingRetainAndReleaseOperations(); 1339 performPendingRetainAndReleaseOperations(); 1339 1340 1340 1341 for (unsigned i = 0; i < urls.size(); ++i) { … … 1419 1420 break; 1420 1421 1421 if (m_retainOrReleaseIconRequested){1422 { 1422 1423 MutexLocker locker(m_urlAndIconLock); 1423 // Previous flag check was done outside of the lock and flag could be changed by another thread. 1424 // Do not move mutex outside to avoid unnecessary locking on every loop, but recheck the flag under mutex. 1425 if (m_retainOrReleaseIconRequested) 1426 performPendingRetainAndReleaseOperations(); 1424 performPendingRetainAndReleaseOperations(); 1427 1425 } 1428 1426 … … 1512 1510 void IconDatabase::performPendingRetainAndReleaseOperations() 1513 1511 { 1514 ASSERT(m_retainOrReleaseIconRequested);1515 1516 1512 // NOTE: The caller is assumed to hold m_urlAndIconLock. 1517 1513 ASSERT(!m_urlAndIconLock.tryLock()); 1518 1514 1519 MutexLocker vectorLocker(m_urlsToRetainOrReleaseLock); 1520 1521 for (HashCountedSet<String>::const_iterator it = m_urlsToRetain.begin(), end = m_urlsToRetain.end(); it != end; ++it) 1515 HashCountedSet<String> toRetain; 1516 HashCountedSet<String> toRelease; 1517 1518 { 1519 MutexLocker pendingWorkLocker(m_urlsToRetainOrReleaseLock); 1520 if (!m_retainOrReleaseIconRequested) 1521 return; 1522 1523 // Make a copy of the URLs to retain and/or release so we can release m_urlsToRetainOrReleaseLock ASAP. 1524 // Holding m_urlAndIconLock protects this function from being re-entered. 1525 1526 toRetain.swap(m_urlsToRetain); 1527 toRelease.swap(m_urlsToRelease); 1528 m_retainOrReleaseIconRequested = false; 1529 } 1530 1531 for (HashCountedSet<String>::const_iterator it = toRetain.begin(), end = toRetain.end(); it != end; ++it) 1522 1532 performRetainIconForPageURL(it->first, it->second); 1523 for (HashCountedSet<String>::const_iterator it = m_urlsToRelease.begin(), end = m_urlsToRelease.end(); it != end; ++it)1533 for (HashCountedSet<String>::const_iterator it = toRelease.begin(), end = toRelease.end(); it != end; ++it) 1524 1534 performReleaseIconForPageURL(it->first, it->second); 1525 1526 m_urlsToRetain.clear();1527 m_urlsToRelease.clear();1528 m_retainOrReleaseIconRequested = false;1529 1535 } 1530 1536 -
trunk/Source/WebCore/loader/icon/IconDatabase.h
r117501 r117744 166 166 bool m_syncThreadHasWorkToDo; 167 167 bool m_disabledSuddenTerminationForSyncThread; 168 bool m_retainOrReleaseIconRequested;169 168 170 169 Mutex m_urlAndIconLock; … … 189 188 HashCountedSet<String> m_urlsToRetain; 190 189 HashCountedSet<String> m_urlsToRelease; 190 bool m_retainOrReleaseIconRequested; 191 191 192 192 // *** Sync Thread Only ***
Note: See TracChangeset
for help on using the changeset viewer.