Changeset 109271 in webkit


Ignore:
Timestamp:
Feb 29, 2012 3:03:19 PM (12 years ago)
Author:
jsbell@chromium.org
Message:

IndexedDB: Resource leak with IDBObjectStore.openCursor
​https://bugs.webkit.org/show_bug.cgi?id=79835

Source/WebCore:

IDBCursor object that were never continue()'d to the end would leak due to a
reference cycle with IDBRequest. In addition, the IDBRequest would never be
marked "finished" which would prevent GC from reclaiming it. IDBTransactions
now track and notify IDBCursors to break these references when the transaction
can no longer not process requests.

Reviewed by Tony Chang.

Tests: storage/indexeddb/cursor-continue.html

  • storage/IDBCursor.cpp:

(WebCore::IDBCursor::IDBCursor): Register with IDBTransaction bookkeeping.
(WebCore::IDBCursor::continueFunction): Early error if transaction has finished.
(WebCore::IDBCursor::close): Break the reference cycle with IDBRequest, and notify it
that the cursor is finished.
(WebCore):

  • storage/IDBCursor.h:

(WebCore):
(IDBCursor):

  • storage/IDBRequest.cpp:

(WebCore::IDBRequest::IDBRequest):
(WebCore::IDBRequest::finishCursor): If there is no request in flight, mark itself as
finished to allow GC.
(WebCore):
(WebCore::IDBRequest::dispatchEvent): Once an in-flight request has been processed,
mark the request as finished if the cursor is finished, to allow GC.

  • storage/IDBRequest.h:

(IDBRequest):

  • storage/IDBTransaction.cpp: Track open cursors, close them when finished.

(WebCore::IDBTransaction::OpenCursorNotifier::OpenCursorNotifier):
(WebCore):
(WebCore::IDBTransaction::OpenCursorNotifier::~OpenCursorNotifier):
(WebCore::IDBTransaction::registerOpenCursor):
(WebCore::IDBTransaction::unregisterOpenCursor):
(WebCore::IDBTransaction::closeOpenCursors):
(WebCore::IDBTransaction::onAbort):
(WebCore::IDBTransaction::onComplete):

  • storage/IDBTransaction.h:

(WebCore):
(OpenCursorNotifier):
(IDBTransaction):

LayoutTests:

Ensure that IDBCursor.continue() throws the right exception when transaction is finished.

Reviewed by Tony Chang.

  • storage/indexeddb/cursor-continue-expected.txt:
  • storage/indexeddb/cursor-continue.html:
Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r109270 r109271  
     12012-02-29  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: Resource leak with IDBObjectStore.openCursor
     4        https://bugs.webkit.org/show_bug.cgi?id=79835
     5
     6        Ensure that IDBCursor.continue() throws the right exception when transaction is finished.
     7
     8        Reviewed by Tony Chang.
     9
     10        * storage/indexeddb/cursor-continue-expected.txt:
     11        * storage/indexeddb/cursor-continue.html:
     12
    1132012-02-29  Adam Klein  <adamk@chromium.org>
    214

  • trunk/LayoutTests/storage/indexeddb/cursor-continue-expected.txt

    r104865 r109271  
    109109event.target.result.continue('A bit2')
    110110PASS event.target.result.primaryKey is 15
     111cursor = event.target.result
    111112Expecting exception from event.target.result.continue('A bit2')
    112113PASS Exception was thrown.
    113114PASS code is IDBDatabaseException.DATA_ERR
     115Expecting exception from cursor.continue()
     116PASS Exception was thrown.
     117PASS code is IDBDatabaseException.TRANSACTION_INACTIVE_ERR
    114118PASS successfullyParsed is true
    115119

  • trunk/LayoutTests/storage/indexeddb/cursor-continue.html

    r104865 r109271  
    263263        } else if (window.stage == 1) {
    264264            shouldBe("event.target.result.primaryKey", "15");
     265            evalAndLog("cursor = event.target.result");
    265266            evalAndExpectException("event.target.result.continue('A bit2')", "IDBDatabaseException.DATA_ERR");
    266             done();
    267             return;
    268         } else {
    269            testFailed("Illegal stage.");
    270         }
    271         window.stage++;
    272     };
     267            window.trans.oncomplete = onTransactionComplete;
     268            return;
     269        } else {
     270           testFailed("Illegal stage.");
     271        }
     272        window.stage++;
     273    };
     274}
     275
     276function onTransactionComplete()
     277{
     278    evalAndExpectException("cursor.continue()", "IDBDatabaseException.TRANSACTION_INACTIVE_ERR");
     279    done();
    273280}
    274281

  • trunk/Source/WebCore/ChangeLog

    r109267 r109271  
     12012-02-29  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: Resource leak with IDBObjectStore.openCursor
     4        https://bugs.webkit.org/show_bug.cgi?id=79835
     5
     6        IDBCursor object that were never continue()'d to the end would leak due to a
     7        reference cycle with IDBRequest. In addition, the IDBRequest would never be
     8        marked "finished" which would prevent GC from reclaiming it. IDBTransactions
     9        now track and notify IDBCursors to break these references when the transaction
     10        can no longer not process requests.
     11
     12        Reviewed by Tony Chang.
     13
     14        Tests: storage/indexeddb/cursor-continue.html
     15
     16        * storage/IDBCursor.cpp:
     17        (WebCore::IDBCursor::IDBCursor): Register with IDBTransaction bookkeeping.
     18        (WebCore::IDBCursor::continueFunction): Early error if transaction has finished.
     19        (WebCore::IDBCursor::close): Break the reference cycle with IDBRequest, and notify it
     20        that the cursor is finished.
     21        (WebCore):
     22        * storage/IDBCursor.h:
     23        (WebCore):
     24        (IDBCursor):
     25        * storage/IDBRequest.cpp:
     26        (WebCore::IDBRequest::IDBRequest):
     27        (WebCore::IDBRequest::finishCursor): If there is no request in flight, mark itself as
     28        finished to allow GC.
     29        (WebCore):
     30        (WebCore::IDBRequest::dispatchEvent): Once an in-flight request has been processed,
     31        mark the request as finished if the cursor is finished, to allow GC.
     32        * storage/IDBRequest.h:
     33        (IDBRequest):
     34        * storage/IDBTransaction.cpp: Track open cursors, close them when finished.
     35        (WebCore::IDBTransaction::OpenCursorNotifier::OpenCursorNotifier):
     36        (WebCore):
     37        (WebCore::IDBTransaction::OpenCursorNotifier::~OpenCursorNotifier):
     38        (WebCore::IDBTransaction::registerOpenCursor):
     39        (WebCore::IDBTransaction::unregisterOpenCursor):
     40        (WebCore::IDBTransaction::closeOpenCursors):
     41        (WebCore::IDBTransaction::onAbort):
     42        (WebCore::IDBTransaction::onComplete):
     43        * storage/IDBTransaction.h:
     44        (WebCore):
     45        (OpenCursorNotifier):
     46        (IDBTransaction):
     47
    1482012-02-29  David Hyatt  <hyatt@apple.com>
    249

  • trunk/Source/WebCore/storage/IDBCursor.cpp

    r108520 r109271  
    5151    , m_source(source)
    5252    , m_transaction(transaction)
     53    , m_transactionNotifier(transaction, this)
    5354{
    5455    ASSERT(m_backend);
    … …  
    111112    }
    112113
     114    if (!m_request) {
     115        ec = IDBDatabaseException::TRANSACTION_INACTIVE_ERR;
     116        return;
     117    }
    113118    // FIXME: We're not using the context from when continue was called, which means the callback
    114119    //        will be on the original context openCursor was called on. Is this right?
    … …  
    137142}
    138143
     144void IDBCursor::close()
     145{
     146    ASSERT(m_request);
     147    m_request->finishCursor();
     148    m_request.clear();
     149}
     150
    139151} // namespace WebCore
    140152

  • trunk/Source/WebCore/storage/IDBCursor.h

    r101645 r109271  
    3030
    3131#include "IDBKey.h"
     32#include "IDBTransaction.h"
    3233#include <wtf/PassRefPtr.h>
    3334#include <wtf/RefCounted.h>
    … …  
    4041class IDBCursorBackendInterface;
    4142class IDBRequest;
    42 class IDBTransaction;
    4343class ScriptExecutionContext;
    4444class SerializedScriptValue;
    … …  
    7272
    7373    void postSuccessHandlerCallback();
     74    void close();
    7475
    7576protected:
    … …  
    8182    RefPtr<IDBAny> m_source;
    8283    RefPtr<IDBTransaction> m_transaction;
     84    IDBTransaction::OpenCursorNotifier m_transactionNotifier;
    8385};
    8486

  • trunk/Source/WebCore/storage/IDBRequest.cpp

    r108520 r109271  
    5959    , m_readyState(LOADING)
    6060    , m_requestFinished(false)
     61    , m_cursorFinished(false)
    6162    , m_contextStopped(false)
    6263    , m_cursorType(IDBCursorBackendInterface::InvalidCursorType)
    … …  
    181182    ASSERT(!m_cursor);
    182183    m_cursor = cursor;
     184}
     185
     186void IDBRequest::finishCursor()
     187{
     188    m_cursorFinished = true;
     189    if (m_readyState != LOADING)
     190        m_requestFinished = true;
    183191}
    184192
    … …  
    365373
    366374    // If the result was of type IDBCursor, or a onBlocked event, then we'll fire again.
    367     if (event->type() != eventNames().blockedEvent && m_result && m_result->type() != IDBAny::IDBCursorType && m_result->type() != IDBAny::IDBCursorWithValueType)
     375    if (event->type() != eventNames().blockedEvent && (!cursorToNotify || m_cursorFinished))
    368376        m_requestFinished = true;
    369377

  • trunk/Source/WebCore/storage/IDBRequest.h

    r101645 r109271  
    7373    void setCursorType(IDBCursorBackendInterface::CursorType);
    7474    void setCursor(PassRefPtr<IDBCursor>);
     75    void finishCursor();
    7576    IDBAny* source();
    7677    void abort();
    … …  
    123124    ReadyState m_readyState;
    124125    bool m_requestFinished; // Is it possible that we'll fire any more events? If not, we're finished.
     126    bool m_cursorFinished;
    125127    bool m_contextStopped;
    126128    Vector<RefPtr<Event> > m_enqueuedEvents;

  • trunk/Source/WebCore/storage/IDBTransaction.cpp

    r108520 r109271  
    127127}
    128128
     129IDBTransaction::OpenCursorNotifier::OpenCursorNotifier(PassRefPtr<IDBTransaction> transaction, IDBCursor* cursor)
     130    : m_transaction(transaction),
     131      m_cursor(cursor)
     132{
     133    m_transaction->registerOpenCursor(m_cursor);
     134}
     135
     136IDBTransaction::OpenCursorNotifier::~OpenCursorNotifier()
     137{
     138    m_transaction->unregisterOpenCursor(m_cursor);
     139}
     140
     141void IDBTransaction::registerOpenCursor(IDBCursor* cursor)
     142{
     143    m_openCursors.add(cursor);
     144}
     145
     146void IDBTransaction::unregisterOpenCursor(IDBCursor* cursor)
     147{
     148    m_openCursors.remove(cursor);
     149}
     150
     151void IDBTransaction::closeOpenCursors()
     152{
     153    HashSet<IDBCursor*> cursors;
     154    cursors.swap(m_openCursors);
     155    for (HashSet<IDBCursor*>::iterator i = cursors.begin(); i != cursors.end(); ++i)
     156        (*i)->close();
     157}
     158
    129159void IDBTransaction::registerRequest(IDBRequest* request)
    130160{
    … …  
    149179    if (m_mode == IDBTransaction::VERSION_CHANGE)
    150180        m_database->clearVersionChangeTransaction(this);
     181    closeOpenCursors();
    151182
    152183    if (m_contextStopped || !scriptExecutionContext())
    … …  
    161192    if (m_mode == IDBTransaction::VERSION_CHANGE)
    162193        m_database->clearVersionChangeTransaction(this);
     194    closeOpenCursors();
    163195
    164196    if (m_contextStopped || !scriptExecutionContext())

  • trunk/Source/WebCore/storage/IDBTransaction.h

    r103283 r109271  
    3737#include "IDBTransactionBackendInterface.h"
    3838#include "IDBTransactionCallbacks.h"
     39#include <wtf/HashSet.h>
    3940#include <wtf/RefCounted.h>
    4041
    4142namespace WebCore {
    4243
     44class IDBCursor;
    4345class IDBDatabase;
    4446class IDBObjectStore;
    … …  
    6264    PassRefPtr<IDBObjectStore> objectStore(const String& name, ExceptionCode&);
    6365    void abort();
     66
     67    class OpenCursorNotifier {
     68    public:
     69        OpenCursorNotifier(PassRefPtr<IDBTransaction>, IDBCursor*);
     70        ~OpenCursorNotifier();
     71    private:
     72        RefPtr<IDBTransaction> m_transaction;
     73        IDBCursor* m_cursor;
     74    };
    6475
    6576    void registerRequest(IDBRequest*);
    … …  
    94105
    95106    void enqueueEvent(PassRefPtr<Event>);
     107    void closeOpenCursors();
     108
     109    void registerOpenCursor(IDBCursor*);
     110    void unregisterOpenCursor(IDBCursor*);
    96111
    97112    // EventTarget
    … …  
    112127    IDBObjectStoreMap m_objectStoreMap;
    113128
     129    HashSet<IDBCursor*> m_openCursors;
     130
    114131    EventTargetData m_eventTargetData;
    115132};
Note: See TracChangeset for help on using the changeset viewer.