Changeset 241499 in webkit
- Timestamp:
- Feb 13, 2019, 10:43:13 PM (6 years ago)
- Location:
- trunk
- Files:
-
- 2 added
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r241497 r241499 1 2019-02-13 Ryosuke Niwa <rniwa@webkit.org> 2 3 Crash in DOMTimer::fired 4 https://bugs.webkit.org/show_bug.cgi?id=194638 5 6 Reviewed by Brent Fulgham. 7 8 Added a regression test. It needs debug assertions without the fix. 9 10 * fast/dom/timer-destruction-during-firing-expected.txt: Added. 11 * fast/dom/timer-destruction-during-firing.html: Added. 12 1 13 2019-02-13 Nikita Vasilyev <nvasilyev@apple.com> 2 14 -
trunk/Source/WebCore/ChangeLog
r241495 r241499 1 2019-02-13 Ryosuke Niwa <rniwa@webkit.org> 2 3 Crash in DOMTimer::fired 4 https://bugs.webkit.org/show_bug.cgi?id=194638 5 6 Reviewed by Brent Fulgham. 7 8 This patch continues the saga of hunting down timer related crashes after r239814, r225985, r227934. 9 10 The crash was caused by the bug that we don't remove a DOMTimer from NestedTimersMap if a DOMTimer 11 is created & installed inside another DOMTimer's callback (via execute call in DOMTimer::fired). 12 13 Fixed the crash by using a Ref in NestedTimersMap. This will keep the timer alive until we exit 14 from DOMTimer::fired. Because DOMTimer::fired always calls stopTracking() which clears the map 15 we would not leak these DOM timers. 16 17 We could, alternatively, use WeakPtr in NestedTimersMap but that would unnecessarily increase the 18 size of DOMTimer for a very marginal benefit of DOMTimer objcets being deleted slightly earlier. 19 Deleting itself in DOMTimer's destructor involves more logic & house keeping in the timer code, 20 and is no longer the preferred approach when dealing with these classes of bugs in WebKit. 21 22 Test: fast/dom/timer-destruction-during-firing.html 23 24 * page/DOMTimer.cpp: 25 (WebCore::NestedTimersMap::add): 26 (WebCore::DOMTimer::install): 27 (WebCore::DOMTimer::fired): 28 1 29 2019-02-13 Joseph Pecoraro <pecoraro@apple.com> 2 30 -
trunk/Source/WebCore/page/DOMTimer.cpp
r239427 r241499 114 114 115 115 struct NestedTimersMap { 116 typedef HashMap<int, DOMTimer*>::const_iterator const_iterator;116 typedef HashMap<int, Ref<DOMTimer>>::const_iterator const_iterator; 117 117 118 118 static NestedTimersMap* instanceForContext(ScriptExecutionContext& context) … … 140 140 } 141 141 142 void add(int timeoutId, DOMTimer*timer)142 void add(int timeoutId, Ref<DOMTimer>&& timer) 143 143 { 144 144 if (isTrackingNestedTimers) 145 nestedTimers.add(timeoutId, timer);145 nestedTimers.add(timeoutId, WTFMove(timer)); 146 146 } 147 147 … … 163 163 164 164 static bool isTrackingNestedTimers; 165 HashMap<int /* timeoutId */, DOMTimer*> nestedTimers;165 HashMap<int /* timeoutId */, Ref<DOMTimer>> nestedTimers; 166 166 }; 167 167 … … 236 236 // Keep track of nested timer installs. 237 237 if (NestedTimersMap* nestedTimers = NestedTimersMap::instanceForContext(context)) 238 nestedTimers->add(timer->m_timeoutId, timer);238 nestedTimers->add(timer->m_timeoutId, *timer); 239 239 240 240 return timer->m_timeoutId; … … 391 391 // Check if we should throttle nested single-shot timers. 392 392 if (nestedTimers) { 393 for (auto& keyValue: *nestedTimers) {394 auto * timer = keyValue.value;393 for (auto& idAndTimer : *nestedTimers) { 394 auto& timer = idAndTimer.value; 395 395 if (timer->isActive() && !timer->repeatInterval()) 396 396 timer->updateThrottlingStateIfNecessary(fireState);
Note:
See TracChangeset
for help on using the changeset viewer.