Changeset 126254 in webkit


Ignore:
Timestamp:
Aug 21, 2012 7:43:47 PM (12 years ago)
Author:
jsbell@chromium.org
Message:

IndexedDB: IDBRequest can be GCd during event dispatch
​https://bugs.webkit.org/show_bug.cgi?id=94235

Reviewed by Ojan Vafai.

Source/WebCore:

Avoid a "race" where GC may attempt to reclaim IDB objects that are marked
"done" prior to the completion of the event dispatch. The script runtime
may decide to do a GC pass before calling the event handler, releasing the
object and turning the dispatch into a no-op.

This is a partial reversion (with renames, etc) of r123275, r124842,
and r121492. Added a new test, although it does not exercise the "race"
condition directly.

Test: storage/indexeddb/pending-activity.html

storage/indexeddb/pending-activity-workers.html

  • Modules/indexeddb/IDBCursor.cpp:

(WebCore::IDBCursor::close): Let the IDBRequest know it this cursor won't
make it fire again.

  • Modules/indexeddb/IDBRequest.cpp:

(WebCore::IDBRequest::IDBRequest): Reintroduce "am I done?" flag.
(WebCore::IDBRequest::finishCursor): Cursors may fire events at the same
IDBRequest repeatedly, so we need to know when they're are really done.
(WebCore):
(WebCore::IDBRequest::hasPendingActivity): Test the flag.
(WebCore::IDBRequest::dispatchEvent): Set the flag.

  • Modules/indexeddb/IDBRequest.h:

(IDBRequest):

  • Modules/indexeddb/IDBTransaction.cpp:

(WebCore::IDBTransaction::IDBTransaction): Reintroduce "am I done?" flag.
(WebCore::IDBTransaction::hasPendingActivity): Test the flag.
(WebCore::IDBTransaction::dispatchEvent): Set the flag.

  • Modules/indexeddb/IDBTransaction.h:

LayoutTests:

Release references to IDBRequest and IDBTransaction objects and force GC,
to ensure that pending events are still fired. (Doesn't exercise race
condition where GC is triggered by script during dispatch itself, though.)

  • storage/indexeddb/pending-activity-expected.txt: Added.
  • storage/indexeddb/pending-activity-workers-expected.txt: Added.
  • storage/indexeddb/pending-activity-workers.html: Added.
  • storage/indexeddb/pending-activity.html: Added.
  • storage/indexeddb/resources/pending-activity.js: Added.

(test):
(prepareDatabase.request.onsuccess.request.onsuccess.request.onsuccess):
(prepareDatabase.request.onsuccess.request.onsuccess):
(prepareDatabase.request.onsuccess):
(prepareDatabase):
(testTransaction):
(transactionOnComplete):
(testRequest):
(requestOnSuccess):
(testCursorRequest):
(cursorRequestOnFirstSuccess):
(cursorRequestOnSecondSuccess):

Location:
trunk
Files:
5 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r126252 r126254  
     12012-08-21  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: IDBRequest can be GCd during event dispatch
     4        https://bugs.webkit.org/show_bug.cgi?id=94235
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Release references to IDBRequest and IDBTransaction objects and force GC,
     9        to ensure that pending events are still fired. (Doesn't exercise race
     10        condition where GC is triggered by script during dispatch itself, though.)
     11
     12        * storage/indexeddb/pending-activity-expected.txt: Added.
     13        * storage/indexeddb/pending-activity-workers-expected.txt: Added.
     14        * storage/indexeddb/pending-activity-workers.html: Added.
     15        * storage/indexeddb/pending-activity.html: Added.
     16        * storage/indexeddb/resources/pending-activity.js: Added.
     17        (test):
     18        (prepareDatabase.request.onsuccess.request.onsuccess.request.onsuccess):
     19        (prepareDatabase.request.onsuccess.request.onsuccess):
     20        (prepareDatabase.request.onsuccess):
     21        (prepareDatabase):
     22        (testTransaction):
     23        (transactionOnComplete):
     24        (testRequest):
     25        (requestOnSuccess):
     26        (testCursorRequest):
     27        (cursorRequestOnFirstSuccess):
     28        (cursorRequestOnSecondSuccess):
     29
    1302012-08-21  Keishi Hattori  <keishi@webkit.org>
    231

  • trunk/Source/WebCore/ChangeLog

    r126253 r126254  
     12012-08-21  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: IDBRequest can be GCd during event dispatch
     4        https://bugs.webkit.org/show_bug.cgi?id=94235
     5
     6        Reviewed by Ojan Vafai.
     7
     8        Avoid a "race" where GC may attempt to reclaim IDB objects that are marked
     9        "done" prior to the completion of the event dispatch. The script runtime
     10        may decide to do a GC pass before calling the event handler, releasing the
     11        object and turning the dispatch into a no-op.
     12
     13        This is a partial reversion (with renames, etc) of r123275, r124842,
     14        and r121492. Added a new test, although it does not exercise the "race"
     15        condition directly.
     16
     17        Test: storage/indexeddb/pending-activity.html
     18              storage/indexeddb/pending-activity-workers.html
     19
     20        * Modules/indexeddb/IDBCursor.cpp:
     21        (WebCore::IDBCursor::close): Let the IDBRequest know it this cursor won't
     22        make it fire again.
     23        * Modules/indexeddb/IDBRequest.cpp:
     24        (WebCore::IDBRequest::IDBRequest): Reintroduce "am I done?" flag.
     25        (WebCore::IDBRequest::finishCursor): Cursors may fire events at the same
     26        IDBRequest repeatedly, so we need to know when they're are really done.
     27        (WebCore):
     28        (WebCore::IDBRequest::hasPendingActivity): Test the flag.
     29        (WebCore::IDBRequest::dispatchEvent): Set the flag.
     30        * Modules/indexeddb/IDBRequest.h:
     31        (IDBRequest):
     32        * Modules/indexeddb/IDBTransaction.cpp:
     33        (WebCore::IDBTransaction::IDBTransaction): Reintroduce "am I done?" flag.
     34        (WebCore::IDBTransaction::hasPendingActivity): Test the flag.
     35        (WebCore::IDBTransaction::dispatchEvent): Set the flag.
     36        * Modules/indexeddb/IDBTransaction.h:
     37
    1382012-08-21  Pavel Feldman  <pfeldman@chromium.org>
    239

  • trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp

    r125568 r126254  
    263263{
    264264    ASSERT(m_request);
     265    m_request->finishCursor();
    265266    m_request.clear();
    266267}

  • trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp

    r126239 r126254  
    6969    , m_source(source)
    7070    , m_taskType(taskType)
     71    , m_hasPendingActivity(true)
    7172    , m_cursorType(IDBCursorBackendInterface::InvalidCursorType)
    7273    , m_cursorDirection(IDBCursor::NEXT)
     74    , m_cursorFinished(false)
    7375    , m_pendingCursor(0)
    7476    , m_didFireUpgradeNeededEvent(false)
    … …  
    223225
    224226    m_result = IDBAny::create(IDBCursorWithValue::fromCursor(cursor));
     227}
     228
     229void IDBRequest::finishCursor()
     230{
     231    m_cursorFinished = true;
    225232}
    226233
    … …  
    405412    //        get a handle to us and we have event listeners. This is order to handle
    406413    //        user generated events properly.
    407     return m_readyState == PENDING || ActiveDOMObject::hasPendingActivity();
     414    return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
    408415}
    409416
    … …  
    439446    ASSERT(m_readyState == PENDING);
    440447    ASSERT(!m_contextStopped);
     448    ASSERT(m_hasPendingActivity);
    441449    ASSERT(m_enqueuedEvents.size());
    442450    ASSERT(scriptExecutionContext());
    … …  
    489497        cursorToNotify->postSuccessHandlerCallback();
    490498
     499    if (m_readyState == DONE && (!cursorToNotify || m_cursorFinished) && event->type() != eventNames().upgradeneededEvent)
     500        m_hasPendingActivity = false;
     501
    491502    if (m_transaction) {
    492503        if (event->type() == eventNames().errorEvent && dontPreventDefault && !m_requestAborted) {

  • trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h

    r125850 r126254  
    7878    void setCursorDetails(IDBCursorBackendInterface::CursorType, IDBCursor::Direction);
    7979    void setPendingCursor(PassRefPtr<IDBCursor>);
     80    void finishCursor();
    8081    void abort();
    8182
    … …  
    139140    const IDBTransactionBackendInterface::TaskType m_taskType;
    140141
     142    bool m_hasPendingActivity;
    141143    Vector<RefPtr<Event> > m_enqueuedEvents;
    142144
    … …  
    144146    IDBCursorBackendInterface::CursorType m_cursorType;
    145147    IDBCursor::Direction m_cursorDirection;
     148    bool m_cursorFinished;
    146149    RefPtr<IDBCursor> m_pendingCursor;
    147150    RefPtr<IDBKey> m_cursorKey;

  • trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp

    r125850 r126254  
    9595    , m_active(true)
    9696    , m_state(Unused)
     97    , m_hasPendingActivity(true)
    9798    , m_contextStopped(false)
    9899{
    … …  
    322323    //        get a handle to us or any child request object and any of those have
    323324    //        event listeners. This is  in order to handle user generated events properly.
    324     return m_state != Finished || ActiveDOMObject::hasPendingActivity();
     325    return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
    325326}
    326327
    … …  
    371372    IDB_TRACE("IDBTransaction::dispatchEvent");
    372373    ASSERT(m_state != Finished);
     374    ASSERT(m_hasPendingActivity);
    373375    ASSERT(scriptExecutionContext());
    374376    ASSERT(event->target() == this);
    … …  
    393395        m_openDBRequest->transactionDidFinishAndDispatch();
    394396    }
     397    m_hasPendingActivity = false;
    395398    return returnValue;
    396399}

  • trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h

    r125850 r126254  
    149149    bool m_active;
    150150    State m_state;
     151    bool m_hasPendingActivity;
    151152    bool m_contextStopped;
    152153    RefPtr<DOMError> m_error;
Note: See TracChangeset for help on using the changeset viewer.