[V8] IndexedDB: IDBTransaction objects leak in Workers
authorjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Feb 2012 00:26:48 +0000 (00:26 +0000)
committerjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 Feb 2012 00:26:48 +0000 (00:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79308

Source/WebCore:

Use a V8RecursionScope whenever Workers call into script, so that
post-script side-effects can be executed.

Reviewed by Tony Chang.

Test: storage/indexeddb/transaction-abort-workers.html

* bindings/v8/ScheduledAction.cpp:
(WebCore::ScheduledAction::execute):
* bindings/v8/V8WorkerContextErrorHandler.cpp:
(WebCore::V8WorkerContextErrorHandler::callListenerFunction):
* bindings/v8/V8WorkerContextEventListener.cpp:
(WebCore::V8WorkerContextEventListener::callListenerFunction):
* bindings/v8/WorkerContextExecutionProxy.cpp: Replace custom recursion tracking.
(WebCore::WorkerContextExecutionProxy::WorkerContextExecutionProxy):
(WebCore::WorkerContextExecutionProxy::runScript):
* bindings/v8/WorkerContextExecutionProxy.h:
(WorkerContextExecutionProxy):

LayoutTests:

Reviewed by Tony Chang.

* fast/js/resources/js-test-pre.js:
(startWorker): Return Worker object so onerror can be hooked.
* storage/indexeddb/resources/transaction-abort-workers.js: Added.
* storage/indexeddb/transaction-abort-workers-expected.txt: Added.
* storage/indexeddb/transaction-abort-workers.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@108867 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/js/resources/js-test-pre.js
LayoutTests/storage/indexeddb/resources/transaction-abort-workers.js [new file with mode: 0644]
LayoutTests/storage/indexeddb/transaction-abort-workers-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/transaction-abort-workers.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/bindings/v8/ScheduledAction.cpp
Source/WebCore/bindings/v8/V8WorkerContextErrorHandler.cpp
Source/WebCore/bindings/v8/V8WorkerContextEventListener.cpp
Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp
Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h

index 5a88994..85c8cb3 100644 (file)
@@ -1,3 +1,16 @@
+2012-02-24  Joshua Bell  <jsbell@chromium.org>
+
+        [V8] IndexedDB: IDBTransaction objects leak in Workers
+        https://bugs.webkit.org/show_bug.cgi?id=79308
+
+        Reviewed by Tony Chang.
+
+        * fast/js/resources/js-test-pre.js:
+        (startWorker): Return Worker object so onerror can be hooked.
+        * storage/indexeddb/resources/transaction-abort-workers.js: Added.
+        * storage/indexeddb/transaction-abort-workers-expected.txt: Added.
+        * storage/indexeddb/transaction-abort-workers.html: Added.
+
 2012-02-24  Adrienne Walker  <enne@google.com>
 
         [chromium] Unreviewed, mark fast/files/workers tests as flaky crashers
index 492ff8c..5bcb34f 100644 (file)
@@ -498,6 +498,8 @@ function startWorker(testScriptURL)
         debug('Got error from worker: ' + event.message);
         finishJSTest();
     }
+
+    return worker;
 }
 
 if (isWorker()) {
diff --git a/LayoutTests/storage/indexeddb/resources/transaction-abort-workers.js b/LayoutTests/storage/indexeddb/resources/transaction-abort-workers.js
new file mode 100644 (file)
index 0000000..191b4ea
--- /dev/null
@@ -0,0 +1,132 @@
+if (this.importScripts) {
+    importScripts('../../../fast/js/resources/js-test-pre.js');
+    importScripts('shared.js');
+}
+
+description("Test IndexedDB workers, recursion, and transaction termination.");
+
+function test()
+{
+    shouldBeTrue("'webkitIndexedDB' in self");
+    shouldBeFalse("webkitIndexedDB == null");
+
+    shouldBeTrue("'webkitIDBCursor' in self");
+    shouldBeFalse("webkitIDBCursor == null");
+
+    evalAndLog("request = webkitIndexedDB.open('transaction-abort-worker')");
+    request.onerror = unexpectedErrorCallback;
+    request.onsuccess = function () {
+        evalAndLog("db = request.result");
+        evalAndLog("request = db.setVersion('1')");
+        request.onerror = unexpectedErrorCallback;
+        request.onblocked = unexpectedBlockedCallback;
+        request.onsuccess = function () {
+            deleteAllObjectStores(db);
+            evalAndLog("trans = request.result");
+            evalAndLog("db.createObjectStore('store')");
+            trans.oncomplete = createTransaction;
+        };
+    };
+}
+
+function createTransaction()
+{
+    debug("");
+    debug("createTransaction():");
+    evalAndLog("transaction = db.transaction('store')");
+    evalAndLog("store = transaction.objectStore('store')");
+    transaction.oncomplete = unexpectedCompleteCallback;
+    transaction.onerror = unexpectedErrorCallback;
+    transaction.onabort = transactionAborted;
+}
+
+function transactionAborted()
+{
+    testPassed("Transaction aborted");
+    evalAndExpectException("store.get(0)", "webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR");
+    recursionTest();
+}
+
+function recursionTest()
+{
+    debug("");
+    debug("recursionTest():");
+    evalAndLog("transaction = db.transaction('store')");
+    evalAndLog("store = transaction.objectStore('store')");
+    transaction.oncomplete = transactionCompleted;
+    transaction.onabort = unexpectedAbortCallback;
+    evalAndLog("store.get(0)");
+    testPassed("transaction is active");
+    recurse(1);
+}
+
+function recurse(count)
+{
+    debug("recursion depth: " + count);
+    evalAndLog("store.get(0)");
+    testPassed("transaction is still active");
+    if (count < 3) {
+        recurse(count + 1);
+    }
+    debug("recursion depth: " + count);
+    evalAndLog("store.get(0)");
+    testPassed("transaction is still active");
+}
+
+function transactionCompleted()
+{
+    testPassed("transaction completed");
+    evalAndExpectException("store.get(0)", "webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR");
+
+    debug("");
+    debug("trying a timeout callback:");
+    evalAndLog("setTimeout(timeoutTest, 0)");
+}
+
+function timeoutTest()
+{
+    debug("");
+    debug("timeoutTest():");
+
+    evalAndLog("transaction = db.transaction('store')");
+    evalAndLog("store = transaction.objectStore('store')");
+    transaction.oncomplete = unexpectedCompleteCallback;
+    transaction.onabort = function () {
+        testPassed("transaction started in setTimeout() callback aborted");
+        evalAndExpectException("store.get(0)", "webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR");
+
+        errorTest();
+    };
+}
+
+function errorTest()
+{
+    debug("");
+    debug("errorTest():");
+    evalAndLog("self.old_onerror = self.onerror");
+    evalAndLog("self.onerror = errorHandler");
+    throw new Error("ignore this");
+}
+
+function errorHandler(e)
+{
+    debug("");
+    debug("errorHandler():");
+    // FIXME: Should be able to stop the error here, but it isn't an Event object.
+    // evalAndLog("event.preventDefault()");
+    evalAndLog("self.onerror = self.old_onerror");
+    evalAndLog("transaction = db.transaction('store')");
+    evalAndLog("store = transaction.objectStore('store')");
+    transaction.oncomplete = unexpectedCompleteCallback;
+    transaction.onerror = unexpectedErrorCallback;
+    transaction.onabort = errorTransactionAborted;
+}
+
+function errorTransactionAborted()
+{
+    testPassed("Transaction aborted");
+    evalAndExpectException("store.get(0)", "webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR");
+    finishJSTest();
+}
+
+test();
diff --git a/LayoutTests/storage/indexeddb/transaction-abort-workers-expected.txt b/LayoutTests/storage/indexeddb/transaction-abort-workers-expected.txt
new file mode 100644 (file)
index 0000000..dc40202
--- /dev/null
@@ -0,0 +1,82 @@
+[Worker] Test IndexedDB workers, recursion, and transaction termination.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Starting worker: resources/transaction-abort-workers.js
+PASS [Worker] 'webkitIndexedDB' in self is true
+PASS [Worker] webkitIndexedDB == null is false
+PASS [Worker] 'webkitIDBCursor' in self is true
+PASS [Worker] webkitIDBCursor == null is false
+[Worker] request = webkitIndexedDB.open('transaction-abort-worker')
+[Worker] db = request.result
+[Worker] request = db.setVersion('1')
+[Worker] Deleted all object stores.
+[Worker] trans = request.result
+[Worker] db.createObjectStore('store')
+[Worker] 
+[Worker] createTransaction():
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+PASS [Worker] Transaction aborted
+[Worker] Expecting exception from store.get(0)
+PASS [Worker] Exception was thrown.
+PASS [Worker] code is webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR
+[Worker] 
+[Worker] recursionTest():
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+[Worker] store.get(0)
+PASS [Worker] transaction is active
+[Worker] recursion depth: 1
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+[Worker] recursion depth: 2
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+[Worker] recursion depth: 3
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+[Worker] recursion depth: 3
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+[Worker] recursion depth: 2
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+[Worker] recursion depth: 1
+[Worker] store.get(0)
+PASS [Worker] transaction is still active
+PASS [Worker] transaction completed
+[Worker] Expecting exception from store.get(0)
+PASS [Worker] Exception was thrown.
+PASS [Worker] code is webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR
+[Worker] 
+[Worker] trying a timeout callback:
+[Worker] setTimeout(timeoutTest, 0)
+[Worker] 
+[Worker] timeoutTest():
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+PASS [Worker] transaction started in setTimeout() callback aborted
+[Worker] Expecting exception from store.get(0)
+PASS [Worker] Exception was thrown.
+PASS [Worker] code is webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR
+[Worker] 
+[Worker] errorTest():
+[Worker] self.old_onerror = self.onerror
+[Worker] self.onerror = errorHandler
+[Worker] 
+[Worker] errorHandler():
+[Worker] self.onerror = self.old_onerror
+[Worker] transaction = db.transaction('store')
+[Worker] store = transaction.objectStore('store')
+Got expected error from worker, ignoring
+event.preventDefault()
+PASS [Worker] Transaction aborted
+[Worker] Expecting exception from store.get(0)
+PASS [Worker] Exception was thrown.
+PASS [Worker] code is webkitIDBDatabaseException.TRANSACTION_INACTIVE_ERR
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/transaction-abort-workers.html b/LayoutTests/storage/indexeddb/transaction-abort-workers.html
new file mode 100644 (file)
index 0000000..c521aad
--- /dev/null
@@ -0,0 +1,31 @@
+<html>
+<head>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+
+var worker = startWorker('resources/transaction-abort-workers.js');
+
+// FIXME: It should be possible for the worker to set self.onerror to catch the event
+// and call event.preventDefault(), but in the current Worker implementation the raw
+// exception is seen by the event handler in the worker, not an ErrorEvent object.
+
+var orig_onerror = worker.onerror;
+worker.onerror = function (event) {
+    if (event.message === "Uncaught Error: ignore this") {
+        debug("Got expected error from worker, ignoring");
+        evalAndLog("event.preventDefault()");
+    } else if (orig_onerror) {
+        orig_onerror(event);
+    }
+};
+
+
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index ab3d5f9..8979de6 100644 (file)
@@ -1,3 +1,27 @@
+2012-02-24  Joshua Bell  <jsbell@chromium.org>
+
+        [V8] IndexedDB: IDBTransaction objects leak in Workers
+        https://bugs.webkit.org/show_bug.cgi?id=79308
+
+        Use a V8RecursionScope whenever Workers call into script, so that
+        post-script side-effects can be executed.
+
+        Reviewed by Tony Chang.
+
+        Test: storage/indexeddb/transaction-abort-workers.html
+
+        * bindings/v8/ScheduledAction.cpp:
+        (WebCore::ScheduledAction::execute):
+        * bindings/v8/V8WorkerContextErrorHandler.cpp:
+        (WebCore::V8WorkerContextErrorHandler::callListenerFunction):
+        * bindings/v8/V8WorkerContextEventListener.cpp:
+        (WebCore::V8WorkerContextEventListener::callListenerFunction):
+        * bindings/v8/WorkerContextExecutionProxy.cpp: Replace custom recursion tracking.
+        (WebCore::WorkerContextExecutionProxy::WorkerContextExecutionProxy):
+        (WebCore::WorkerContextExecutionProxy::runScript):
+        * bindings/v8/WorkerContextExecutionProxy.h:
+        (WorkerContextExecutionProxy):
+
 2012-02-24  Gregg Tavares  <gman@google.com>
 
         Limit WebGL Errors in Console to 10 per context
index f76b58d..c406047 100644 (file)
@@ -41,6 +41,7 @@
 
 #include "V8Binding.h"
 #include "V8Proxy.h"
+#include "V8RecursionScope.h"
 #include "WorkerContext.h"
 #include "WorkerContextExecutionProxy.h"
 #include "WorkerThread.h"
@@ -141,7 +142,8 @@ void ScheduledAction::execute(WorkerContext* workerContext)
 {
     // In a Worker, the execution should always happen on a worker thread.
     ASSERT(workerContext->thread()->threadID() == currentThread());
-  
+
+    V8RecursionScope recursionScope(workerContext);
     WorkerScriptController* scriptController = workerContext->script();
 
     if (!m_function.IsEmpty() && m_function->IsFunction()) {
index d02278e..b9bdecd 100644 (file)
@@ -37,6 +37,7 @@
 #include "EventNames.h"
 #include "ErrorEvent.h"
 #include "V8Binding.h"
+#include "V8RecursionScope.h"
 
 namespace WebCore {
 
@@ -55,6 +56,7 @@ v8::Local<v8::Value> V8WorkerContextErrorHandler::callListenerFunction(ScriptExe
         v8::Local<v8::Function> callFunction = v8::Local<v8::Function>::Cast(listener);
         v8::Local<v8::Object> thisValue = v8::Context::GetCurrent()->Global();
         v8::Handle<v8::Value> parameters[3] = { v8String(errorEvent->message()), v8String(errorEvent->filename()), v8::Integer::New(errorEvent->lineno()) };
+        V8RecursionScope recursionScope(context);
         returnValue = callFunction->Call(thisValue, 3, parameters);
     }
     return returnValue;
index 30b9865..7fd4f3f 100644 (file)
@@ -37,6 +37,7 @@
 #include "V8Binding.h"
 #include "V8DOMWrapper.h"
 #include "V8Event.h"
+#include "V8RecursionScope.h"
 #include "WorkerContext.h"
 #include "WorkerContextExecutionProxy.h"
 
@@ -90,6 +91,7 @@ v8::Local<v8::Value> V8WorkerContextEventListener::callListenerFunction(ScriptEx
         return v8::Local<v8::Value>();
 
     v8::Handle<v8::Value> parameters[1] = { jsEvent };
+    V8RecursionScope recursionScope(context);
     v8::Local<v8::Value> result = handlerFunction->Call(receiver, 1, parameters);
 
     if (WorkerContextExecutionProxy* proxy = workerProxy(context))
index d2840bd..51be5ca 100644 (file)
@@ -44,6 +44,7 @@
 #include "V8DOMMap.h"
 #include "V8DedicatedWorkerContext.h"
 #include "V8Proxy.h"
+#include "V8RecursionScope.h"
 #include "V8SharedWorkerContext.h"
 #include "Worker.h"
 #include "WorkerContext.h"
@@ -81,7 +82,6 @@ static void v8MessageHandler(v8::Handle<v8::Message> message, v8::Handle<v8::Val
 
 WorkerContextExecutionProxy::WorkerContextExecutionProxy(WorkerContext* workerContext)
     : m_workerContext(workerContext)
-    , m_recursion(0)
 {
     initIsolate();
 }
@@ -234,7 +234,7 @@ v8::Local<v8::Value> WorkerContextExecutionProxy::runScript(v8::Handle<v8::Scrip
         return v8::Local<v8::Value>();
 
     // Compute the source string and prevent against infinite recursion.
-    if (m_recursion >= kMaxRecursionDepth) {
+    if (V8RecursionScope::recursionLevel() >= kMaxRecursionDepth) {
         v8::Local<v8::String> code = v8ExternalString("throw RangeError('Recursion too deep')");
         script = V8Proxy::compileScript(code, "", TextPosition::minimumPosition());
     }
@@ -248,9 +248,8 @@ v8::Local<v8::Value> WorkerContextExecutionProxy::runScript(v8::Handle<v8::Scrip
     // Run the script and keep track of the current recursion depth.
     v8::Local<v8::Value> result;
     {
-        m_recursion++;
+        V8RecursionScope recursionScope(m_workerContext);
         result = script->Run();
-        m_recursion--;
     }
 
     // Handle V8 internal error situation (Out-of-memory).
index 7d4d70c..e6c1605 100644 (file)
@@ -86,7 +86,6 @@ namespace WebCore {
 
         WorkerContext* m_workerContext;
         v8::Persistent<v8::Context> m_context;
-        int m_recursion;
 
         Vector<Event*> m_events;
     };