Title: [217307] trunk/Source/WebKit2
- Revision
- 217307
- Author
- cdu...@apple.com
- Date
- 2017-05-23 16:45:41 -0700 (Tue, 23 May 2017)
Log Message
ASSERT(!m_timeoutTimer.isActive()) hit in BackgroundProcessResponsivenessTimer::responsivenessCheckTimerFired()
https://bugs.webkit.org/show_bug.cgi?id=172509
<rdar://problem/32251578>
Reviewed by Geoffrey Garen.
In the BackgroundProcessResponsivenessTimer class, we have 2 timers:
- m_responsivenessCheckTimer that causes us to do an IPC handshake with
the WebProcess.
- m_timeoutTimer that is started when we send the IPC message to the
WebProcess and which is stopped when we get the response from the
WebProcess. If we do not get the response by the time m_timeoutTimer
fires, then we mark the process as unresponsive.
As a result, of the behavior above, whenever the BackgroundProcessResponsivenessTimer
is considered "active", there should be one of the 2 timers above that
is active, and only one.
The assertion hit showed that we decided to start the m_responsivenessCheckTimer
timer even though the m_timeoutTimer timer is still active (we are still waiting
for an IPC message from the WebProcess and the process is not considered
unresponsive yet), which is wrong. The reason was that in
BackgroundProcessResponsivenessTimer::updateState(), if we should be active,
we would start the m_responsivenessCheckTimer if m_responsivenessCheckTimer is
not already active, without checking if m_timeoutTimer is active. So if
updateState() was called while the IPC handshake was in process, we would have
both timers running at the same time.
* UIProcess/BackgroundProcessResponsivenessTimer.cpp:
(WebKit::BackgroundProcessResponsivenessTimer::updateState):
(WebKit::BackgroundProcessResponsivenessTimer::isActive):
* UIProcess/BackgroundProcessResponsivenessTimer.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (217306 => 217307)
--- trunk/Source/WebKit2/ChangeLog 2017-05-23 22:58:23 UTC (rev 217306)
+++ trunk/Source/WebKit2/ChangeLog 2017-05-23 23:45:41 UTC (rev 217307)
@@ -1,3 +1,38 @@
+2017-05-23 Chris Dumez <cdu...@apple.com>
+
+ ASSERT(!m_timeoutTimer.isActive()) hit in BackgroundProcessResponsivenessTimer::responsivenessCheckTimerFired()
+ https://bugs.webkit.org/show_bug.cgi?id=172509
+ <rdar://problem/32251578>
+
+ Reviewed by Geoffrey Garen.
+
+ In the BackgroundProcessResponsivenessTimer class, we have 2 timers:
+ - m_responsivenessCheckTimer that causes us to do an IPC handshake with
+ the WebProcess.
+ - m_timeoutTimer that is started when we send the IPC message to the
+ WebProcess and which is stopped when we get the response from the
+ WebProcess. If we do not get the response by the time m_timeoutTimer
+ fires, then we mark the process as unresponsive.
+
+ As a result, of the behavior above, whenever the BackgroundProcessResponsivenessTimer
+ is considered "active", there should be one of the 2 timers above that
+ is active, and only one.
+
+ The assertion hit showed that we decided to start the m_responsivenessCheckTimer
+ timer even though the m_timeoutTimer timer is still active (we are still waiting
+ for an IPC message from the WebProcess and the process is not considered
+ unresponsive yet), which is wrong. The reason was that in
+ BackgroundProcessResponsivenessTimer::updateState(), if we should be active,
+ we would start the m_responsivenessCheckTimer if m_responsivenessCheckTimer is
+ not already active, without checking if m_timeoutTimer is active. So if
+ updateState() was called while the IPC handshake was in process, we would have
+ both timers running at the same time.
+
+ * UIProcess/BackgroundProcessResponsivenessTimer.cpp:
+ (WebKit::BackgroundProcessResponsivenessTimer::updateState):
+ (WebKit::BackgroundProcessResponsivenessTimer::isActive):
+ * UIProcess/BackgroundProcessResponsivenessTimer.h:
+
2017-05-22 Simon Fraser <simon.fra...@apple.com>
Snapshotting via -renderInContext: should do synchronous image decodes
Modified: trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.cpp (217306 => 217307)
--- trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.cpp 2017-05-23 22:58:23 UTC (rev 217306)
+++ trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.cpp 2017-05-23 23:45:41 UTC (rev 217307)
@@ -60,10 +60,8 @@
return;
}
- if (!m_responsivenessCheckTimer.isActive()) {
- ASSERT(!m_timeoutTimer.isActive());
+ if (!isActive())
m_responsivenessCheckTimer.startOneShot(m_checkingInterval);
- }
}
void BackgroundProcessResponsivenessTimer::didReceiveBackgroundResponsivenessPong()
@@ -141,6 +139,11 @@
#endif
}
+bool BackgroundProcessResponsivenessTimer::isActive() const
+{
+ return m_responsivenessCheckTimer.isActive() || m_timeoutTimer.isActive();
+}
+
void BackgroundProcessResponsivenessTimer::scheduleNextResponsivenessCheck()
{
// Exponential backoff to avoid waking up the process too often.
Modified: trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.h (217306 => 217307)
--- trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.h 2017-05-23 22:58:23 UTC (rev 217306)
+++ trunk/Source/WebKit2/UIProcess/BackgroundProcessResponsivenessTimer.h 2017-05-23 23:45:41 UTC (rev 217307)
@@ -50,6 +50,7 @@
void setResponsive(bool);
bool shouldBeActive() const;
+ bool isActive() const;
void scheduleNextResponsivenessCheck();
ResponsivenessTimer::Client& client() const;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
