- Revision
- 126254
- Author
- [email protected]
- Date
- 2012-08-21 19:43:47 -0700 (Tue, 21 Aug 2012)
Log 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):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (126253 => 126254)
--- trunk/LayoutTests/ChangeLog 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/LayoutTests/ChangeLog 2012-08-22 02:43:47 UTC (rev 126254)
@@ -1,3 +1,32 @@
+2012-08-21 Joshua Bell <[email protected]>
+
+ IndexedDB: IDBRequest can be GCd during event dispatch
+ https://bugs.webkit.org/show_bug.cgi?id=94235
+
+ Reviewed by Ojan Vafai.
+
+ 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):
+
2012-08-21 Keishi Hattori <[email protected]>
range-hit-test-with-padding.html fails unless subpixel layout is on
Added: trunk/LayoutTests/storage/indexeddb/pending-activity-expected.txt (0 => 126254)
--- trunk/LayoutTests/storage/indexeddb/pending-activity-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/pending-activity-expected.txt 2012-08-22 02:43:47 UTC (rev 126254)
@@ -0,0 +1,51 @@
+Checks that garbage collection doesn't reclaim objects with pending activity
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+dbname = "pending-activity.html"
+
+prepareDatabase():
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+db = request.result
+PASS db.version is ""
+db.setVersion('1')
+store = db.createObjectStore('store')
+store.put(0, 0)
+
+testTransaction():
+transaction = db.transaction('store')
+transaction._oncomplete_ = transactionOnComplete
+transaction = null
+self.gc()
+PASS Transaction 'complete' event fired.
+
+testRequest():
+transaction = db.transaction('store')
+store = transaction.objectStore('store')
+request = store.get(0)
+request._onsuccess_ = requestOnSuccess
+request = null
+self.gc()
+PASS Request 'success' event fired.
+
+testCursorRequest():
+transaction = db.transaction('store')
+store = transaction.objectStore('store')
+request = store.openCursor()
+request._onsuccess_ = cursorRequestOnFirstSuccess
+PASS Request 'success' event fired.
+cursor = request.result
+cursor.continue()
+request._onsuccess_ = cursorRequestOnSecondSuccess
+request = null
+self.gc()
+PASS Request 'success' event fired.
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/storage/indexeddb/pending-activity-workers-expected.txt (0 => 126254)
--- trunk/LayoutTests/storage/indexeddb/pending-activity-workers-expected.txt (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/pending-activity-workers-expected.txt 2012-08-22 02:43:47 UTC (rev 126254)
@@ -0,0 +1,52 @@
+[Worker] Checks that garbage collection doesn't reclaim objects with pending activity
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Starting worker: resources/pending-activity.js
+[Worker] indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+[Worker]
+[Worker] dbname = "pending-activity.js"
+[Worker]
+[Worker] prepareDatabase():
+[Worker] indexedDB.deleteDatabase(dbname)
+[Worker] indexedDB.open(dbname)
+[Worker] db = request.result
+PASS [Worker] db.version is ""
+[Worker] db.setVersion('1')
+[Worker] store = db.createObjectStore('store')
+[Worker] store.put(0, 0)
+[Worker]
+[Worker] testTransaction():
+[Worker] transaction = db.transaction('store')
+[Worker] transaction._oncomplete_ = transactionOnComplete
+[Worker] transaction = null
+[Worker] self.gc()
+PASS [Worker] Transaction 'complete' event fired.
+[Worker]
+[Worker] testRequest():
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+[Worker] request = store.get(0)
+[Worker] request._onsuccess_ = requestOnSuccess
+[Worker] request = null
+[Worker] self.gc()
+PASS [Worker] Request 'success' event fired.
+[Worker]
+[Worker] testCursorRequest():
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+[Worker] request = store.openCursor()
+[Worker] request._onsuccess_ = cursorRequestOnFirstSuccess
+PASS [Worker] Request 'success' event fired.
+[Worker] cursor = request.result
+[Worker] cursor.continue()
+[Worker] request._onsuccess_ = cursorRequestOnSecondSuccess
+[Worker] request = null
+[Worker] self.gc()
+PASS [Worker] Request 'success' event fired.
+[Worker]
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/storage/indexeddb/pending-activity-workers.html (0 => 126254)
--- trunk/LayoutTests/storage/indexeddb/pending-activity-workers.html (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/pending-activity-workers.html 2012-08-22 02:43:47 UTC (rev 126254)
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<script src=""
+<script src=""
+<body>
+<script>startWorker('resources/pending-activity.js');</script>
+<script src=""
+</body>
Added: trunk/LayoutTests/storage/indexeddb/pending-activity.html (0 => 126254)
--- trunk/LayoutTests/storage/indexeddb/pending-activity.html (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/pending-activity.html 2012-08-22 02:43:47 UTC (rev 126254)
@@ -0,0 +1,5 @@
+<!DOCTYPE html>
+<script src=""
+<script src=""
+<script src=""
+<script src=""
Added: trunk/LayoutTests/storage/indexeddb/resources/pending-activity.js (0 => 126254)
--- trunk/LayoutTests/storage/indexeddb/resources/pending-activity.js (rev 0)
+++ trunk/LayoutTests/storage/indexeddb/resources/pending-activity.js 2012-08-22 02:43:47 UTC (rev 126254)
@@ -0,0 +1,97 @@
+if (this.importScripts) {
+ importScripts('../../../fast/js/resources/js-test-pre.js');
+ importScripts('shared.js');
+}
+
+description("Checks that garbage collection doesn't reclaim objects with pending activity");
+
+function test()
+{
+ removeVendorPrefixes();
+ setDBNameFromPath();
+ prepareDatabase();
+}
+
+function prepareDatabase()
+{
+ preamble();
+ request = evalAndLog("indexedDB.deleteDatabase(dbname)");
+ request._onerror_ = unexpectedErrorCallback;
+ request._onsuccess_ = function(e) {
+ request = evalAndLog("indexedDB.open(dbname)");
+ request._onerror_ = unexpectedErrorCallback;
+ request._onsuccess_ = function(e) {
+ evalAndLog("db = request.result");
+ shouldBeEqualToString("db.version", "");
+ request = evalAndLog("db.setVersion('1')");
+ request._onerror_ = unexpectedErrorCallback;
+ request._onsuccess_ = function(e) {
+ trans = request.result;
+ evalAndLog("store = db.createObjectStore('store')");
+ evalAndLog("store.put(0, 0)");
+ trans._oncomplete_ = testTransaction;
+ };
+ };
+ };
+}
+
+function testTransaction()
+{
+ preamble();
+ evalAndLog("transaction = db.transaction('store')");
+ evalAndLog("transaction._oncomplete_ = transactionOnComplete");
+ evalAndLog("transaction = null");
+ evalAndLog("self.gc()");
+}
+
+function transactionOnComplete()
+{
+ testPassed("Transaction 'complete' event fired.");
+ testRequest();
+}
+
+function testRequest()
+{
+ preamble();
+ evalAndLog("transaction = db.transaction('store')");
+ evalAndLog("store = transaction.objectStore('store')");
+ evalAndLog("request = store.get(0)");
+ evalAndLog("request._onsuccess_ = requestOnSuccess");
+ evalAndLog("request = null");
+ evalAndLog("self.gc()");
+}
+
+function requestOnSuccess()
+{
+ testPassed("Request 'success' event fired.");
+ testCursorRequest();
+}
+
+function testCursorRequest()
+{
+ preamble();
+ evalAndLog("transaction = db.transaction('store')");
+ evalAndLog("store = transaction.objectStore('store')");
+ evalAndLog("request = store.openCursor()");
+ evalAndLog("request._onsuccess_ = cursorRequestOnFirstSuccess");
+}
+
+function cursorRequestOnFirstSuccess()
+{
+ testPassed("Request 'success' event fired.");
+ evalAndLog("cursor = request.result");
+ evalAndLog("cursor.continue()");
+ evalAndLog("request._onsuccess_ = cursorRequestOnSecondSuccess");
+ evalAndLog("request = null");
+ evalAndLog("self.gc()");
+}
+
+function cursorRequestOnSecondSuccess()
+{
+ testPassed("Request 'success' event fired.");
+
+ debug("");
+ finishJSTest();
+}
+
+test();
Modified: trunk/Source/WebCore/ChangeLog (126253 => 126254)
--- trunk/Source/WebCore/ChangeLog 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/ChangeLog 2012-08-22 02:43:47 UTC (rev 126254)
@@ -1,3 +1,40 @@
+2012-08-21 Joshua Bell <[email protected]>
+
+ IndexedDB: IDBRequest can be GCd during event dispatch
+ https://bugs.webkit.org/show_bug.cgi?id=94235
+
+ Reviewed by Ojan Vafai.
+
+ 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:
+
2012-08-21 Pavel Feldman <[email protected]>
Web Inspector: [regression] Settings panel fails to open.
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp (126253 => 126254)
--- trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBCursor.cpp 2012-08-22 02:43:47 UTC (rev 126254)
@@ -262,6 +262,7 @@
void IDBCursor::close()
{
ASSERT(m_request);
+ m_request->finishCursor();
m_request.clear();
}
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp (126253 => 126254)
--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp 2012-08-22 02:43:47 UTC (rev 126254)
@@ -68,8 +68,10 @@
, m_requestAborted(false)
, m_source(source)
, m_taskType(taskType)
+ , m_hasPendingActivity(true)
, m_cursorType(IDBCursorBackendInterface::InvalidCursorType)
, m_cursorDirection(IDBCursor::NEXT)
+ , m_cursorFinished(false)
, m_pendingCursor(0)
, m_didFireUpgradeNeededEvent(false)
{
@@ -224,6 +226,11 @@
m_result = IDBAny::create(IDBCursorWithValue::fromCursor(cursor));
}
+void IDBRequest::finishCursor()
+{
+ m_cursorFinished = true;
+}
+
bool IDBRequest::shouldEnqueueEvent() const
{
if (m_contextStopped || !scriptExecutionContext())
@@ -404,7 +411,7 @@
// FIXME: In an ideal world, we should return true as long as anyone has a or can
// get a handle to us and we have event listeners. This is order to handle
// user generated events properly.
- return m_readyState == PENDING || ActiveDOMObject::hasPendingActivity();
+ return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
}
void IDBRequest::stop()
@@ -438,6 +445,7 @@
IDB_TRACE("IDBRequest::dispatchEvent");
ASSERT(m_readyState == PENDING);
ASSERT(!m_contextStopped);
+ ASSERT(m_hasPendingActivity);
ASSERT(m_enqueuedEvents.size());
ASSERT(scriptExecutionContext());
ASSERT(event->target() == this);
@@ -488,6 +496,9 @@
if (cursorToNotify)
cursorToNotify->postSuccessHandlerCallback();
+ if (m_readyState == DONE && (!cursorToNotify || m_cursorFinished) && event->type() != eventNames().upgradeneededEvent)
+ m_hasPendingActivity = false;
+
if (m_transaction) {
if (event->type() == eventNames().errorEvent && dontPreventDefault && !m_requestAborted) {
m_transaction->setError(m_error);
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h (126253 => 126254)
--- trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBRequest.h 2012-08-22 02:43:47 UTC (rev 126254)
@@ -77,6 +77,7 @@
void markEarlyDeath();
void setCursorDetails(IDBCursorBackendInterface::CursorType, IDBCursor::Direction);
void setPendingCursor(PassRefPtr<IDBCursor>);
+ void finishCursor();
void abort();
// IDBCallbacks
@@ -138,11 +139,13 @@
RefPtr<IDBAny> m_source;
const IDBTransactionBackendInterface::TaskType m_taskType;
+ bool m_hasPendingActivity;
Vector<RefPtr<Event> > m_enqueuedEvents;
// Only used if the result type will be a cursor.
IDBCursorBackendInterface::CursorType m_cursorType;
IDBCursor::Direction m_cursorDirection;
+ bool m_cursorFinished;
RefPtr<IDBCursor> m_pendingCursor;
RefPtr<IDBKey> m_cursorKey;
RefPtr<IDBKey> m_cursorPrimaryKey;
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp (126253 => 126254)
--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp 2012-08-22 02:43:47 UTC (rev 126254)
@@ -94,6 +94,7 @@
, m_mode(mode)
, m_active(true)
, m_state(Unused)
+ , m_hasPendingActivity(true)
, m_contextStopped(false)
{
ASSERT(m_backend);
@@ -321,7 +322,7 @@
// FIXME: In an ideal world, we should return true as long as anyone has a or can
// get a handle to us or any child request object and any of those have
// event listeners. This is in order to handle user generated events properly.
- return m_state != Finished || ActiveDOMObject::hasPendingActivity();
+ return m_hasPendingActivity || ActiveDOMObject::hasPendingActivity();
}
IDBTransaction::Mode IDBTransaction::stringToMode(const String& modeString, ExceptionCode& ec)
@@ -370,6 +371,7 @@
{
IDB_TRACE("IDBTransaction::dispatchEvent");
ASSERT(m_state != Finished);
+ ASSERT(m_hasPendingActivity);
ASSERT(scriptExecutionContext());
ASSERT(event->target() == this);
m_state = Finished;
@@ -392,6 +394,7 @@
ASSERT(isVersionChange());
m_openDBRequest->transactionDidFinishAndDispatch();
}
+ m_hasPendingActivity = false;
return returnValue;
}
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h (126253 => 126254)
--- trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h 2012-08-22 02:39:50 UTC (rev 126253)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h 2012-08-22 02:43:47 UTC (rev 126254)
@@ -148,6 +148,7 @@
const Mode m_mode;
bool m_active;
State m_state;
+ bool m_hasPendingActivity;
bool m_contextStopped;
RefPtr<DOMError> m_error;
