Title: [181219] trunk/Source/WebCore
- Revision
- 181219
- Author
- cdu...@apple.com
- Date
- 2015-03-07 19:37:26 -0800 (Sat, 07 Mar 2015)
Log Message
Crash in WebCore::NotificationCenter::stop()
https://bugs.webkit.org/show_bug.cgi?id=142444
<rdar://problem/20082520>
Reviewed by Andreas Kling.
A use-after-free would sometimes cause us to crash in NotificationCenter::stop().
After investigation, it turns out that NotificationCenter::stop() calls
NotificationClient::clearNotifications() which will destroy the Notification
objects, all of which hold a strong reference to the NotificationCenter. If at
this point, only Notifications are ref'ing the NotificationCenter, this means
that the NotificationCenter will get destroyed right after the call to
NotificationClient::clearNotifications(). However, we reset m_client to null
after calling clearNotifications() and it causes us to crash in this case.
The issue is addressed by adding a Ref<NotificationCenter> protector in
NotificationCenter::stop() so that we make sure the NotificationCenter lives
at least until the end of the method execution.
I was able to consistently reproduce the crash by doing:
Tools/Scripts/run-webkit-tests -1 --debug --repeat-each=30 -g http/tests/notifications/event-listener-crash.html
No new tests, already covered by:
http/tests/notifications/event-listener-crash.html
* Modules/notifications/NotificationCenter.cpp:
(WebCore::NotificationCenter::stop):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (181218 => 181219)
--- trunk/Source/WebCore/ChangeLog 2015-03-08 02:49:21 UTC (rev 181218)
+++ trunk/Source/WebCore/ChangeLog 2015-03-08 03:37:26 UTC (rev 181219)
@@ -1,3 +1,33 @@
+2015-03-07 Chris Dumez <cdu...@apple.com>
+
+ Crash in WebCore::NotificationCenter::stop()
+ https://bugs.webkit.org/show_bug.cgi?id=142444
+ <rdar://problem/20082520>
+
+ Reviewed by Andreas Kling.
+
+ A use-after-free would sometimes cause us to crash in NotificationCenter::stop().
+ After investigation, it turns out that NotificationCenter::stop() calls
+ NotificationClient::clearNotifications() which will destroy the Notification
+ objects, all of which hold a strong reference to the NotificationCenter. If at
+ this point, only Notifications are ref'ing the NotificationCenter, this means
+ that the NotificationCenter will get destroyed right after the call to
+ NotificationClient::clearNotifications(). However, we reset m_client to null
+ after calling clearNotifications() and it causes us to crash in this case.
+
+ The issue is addressed by adding a Ref<NotificationCenter> protector in
+ NotificationCenter::stop() so that we make sure the NotificationCenter lives
+ at least until the end of the method execution.
+
+ I was able to consistently reproduce the crash by doing:
+ Tools/Scripts/run-webkit-tests -1 --debug --repeat-each=30 -g http/tests/notifications/event-listener-crash.html
+
+ No new tests, already covered by:
+ http/tests/notifications/event-listener-crash.html
+
+ * Modules/notifications/NotificationCenter.cpp:
+ (WebCore::NotificationCenter::stop):
+
2015-03-07 Simon Fraser <simon.fra...@apple.com>
Tidy up RenderLayerCompositor's CompositingState
Modified: trunk/Source/WebCore/Modules/notifications/NotificationCenter.cpp (181218 => 181219)
--- trunk/Source/WebCore/Modules/notifications/NotificationCenter.cpp 2015-03-08 02:49:21 UTC (rev 181218)
+++ trunk/Source/WebCore/Modules/notifications/NotificationCenter.cpp 2015-03-08 03:37:26 UTC (rev 181219)
@@ -100,6 +100,11 @@
{
if (!m_client)
return;
+
+ // The call to clearNotifications() below will destroy the notifications. The notifications will
+ // unref the NotificationCenter when destroyed so we need to protect the NotificationCenter here
+ // in case no-one else holds a ref to the NotificationCenter at this point besides Notifications.
+ Ref<NotificationCenter> protect(*this);
m_client->cancelRequestsForPermission(scriptExecutionContext());
m_client->clearNotifications(scriptExecutionContext());
m_client = nullptr;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
