Changeset 181219 in webkit


Ignore:
Timestamp:
Mar 7, 2015 7:37:26 PM (9 years ago)
Author:
Chris Dumez
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):

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r181218 r181219  
     12015-03-07  Chris Dumez  <cdumez@apple.com>
     2
     3        Crash in WebCore::NotificationCenter::stop()
     4        https://bugs.webkit.org/show_bug.cgi?id=142444
     5        <rdar://problem/20082520>
     6
     7        Reviewed by Andreas Kling.
     8
     9        A use-after-free would sometimes cause us to crash in NotificationCenter::stop().
     10        After investigation, it turns out that NotificationCenter::stop() calls
     11        NotificationClient::clearNotifications() which will destroy the Notification
     12        objects, all of which hold a strong reference to the NotificationCenter. If at
     13        this point, only Notifications are ref'ing the NotificationCenter, this means
     14        that the NotificationCenter will get destroyed right after the call to
     15        NotificationClient::clearNotifications(). However, we reset m_client to null
     16        after calling clearNotifications() and it causes us to crash in this case.
     17
     18        The issue is addressed by adding a Ref<NotificationCenter> protector in
     19        NotificationCenter::stop() so that we make sure the NotificationCenter lives
     20        at least until the end of the method execution.
     21
     22        I was able to consistently reproduce the crash by doing:
     23        Tools/Scripts/run-webkit-tests -1 --debug --repeat-each=30 -g http/tests/notifications/event-listener-crash.html
     24
     25        No new tests, already covered by:
     26        http/tests/notifications/event-listener-crash.html
     27
     28        * Modules/notifications/NotificationCenter.cpp:
     29        (WebCore::NotificationCenter::stop):
     30
    1312015-03-07  Simon Fraser  <simon.fraser@apple.com>
    232
  • trunk/Source/WebCore/Modules/notifications/NotificationCenter.cpp

    r180871 r181219  
    101101    if (!m_client)
    102102        return;
     103
     104    // The call to clearNotifications() below will destroy the notifications. The notifications will
     105    // unref the NotificationCenter when destroyed so we need to protect the NotificationCenter here
     106    // in case no-one else holds a ref to the NotificationCenter at this point besides Notifications.
     107    Ref<NotificationCenter> protect(*this);
    103108    m_client->cancelRequestsForPermission(scriptExecutionContext());
    104109    m_client->clearNotifications(scriptExecutionContext());
Note: See TracChangeset for help on using the changeset viewer.