From 4a0018ec63da3ed848497e62169c8782d94cc4b8 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Fri, 1 May 2015 16:54:35 -0400 Subject: [PATCH] Fix transaction detection This fixes an issue where two transactions started around the same time could run separately instead of nesting, causing the statements from one to end up running not within a transaction --- chrome/content/zotero/xpcom/db.js | 10 ++++++--- test/tests/dbTest.js | 36 +++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js index 34d9e0db2..c83712310 100644 --- a/chrome/content/zotero/xpcom/db.js +++ b/chrome/content/zotero/xpcom/db.js @@ -450,9 +450,8 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func } } - var conn = yield this._getConnectionAsync(options); try { - if (conn.transactionInProgress) { + if (this._inTransaction) { Zotero.debug("Async DB transaction in progress -- increasing level to " + ++this._asyncTransactionNestingLevel, 5); @@ -469,7 +468,7 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func var result = yield Zotero.Promise.coroutine(func)(); } catch (e) { - Zotero.debug("Rolled back nested async DB transaction", 5); + Zotero.debug("Rolling back nested async DB transaction", 5); this._asyncTransactionNestingLevel = 0; throw e; } @@ -481,6 +480,8 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func else { Zotero.debug("Beginning async DB transaction", 5); + this._inTransaction = true; + var resolve; var reject; this._transactionPromise = new Zotero.Promise(function () { @@ -497,8 +498,10 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func this._callbacks.begin[i](); } } + var conn = yield this._getConnectionAsync(options); var result = yield conn.executeTransaction(func); Zotero.debug("Committed async DB transaction", 5); + this._inTransaction = false; // Clear transaction time if (this._transactionDate) { @@ -540,6 +543,7 @@ Zotero.DBConnection.prototype.executeTransaction = Zotero.Promise.coroutine(func catch (e) { Zotero.debug("Rolled back async DB transaction", 5); Zotero.debug(e, 1); + this._inTransaction = false; if (options) { // Function to run once transaction has been committed but before any diff --git a/test/tests/dbTest.js b/test/tests/dbTest.js index 5581f0ee9..dd4c59260 100644 --- a/test/tests/dbTest.js +++ b/test/tests/dbTest.js @@ -1,5 +1,41 @@ describe("Zotero.DB", function() { describe("#executeTransaction()", function () { + it("should nest concurrent transactions", Zotero.Promise.coroutine(function* () { + var tmpTable = "tmpWaitForTransactions"; + yield Zotero.DB.queryAsync("CREATE TABLE " + tmpTable + " (foo INT)"); + + var resolve1, resolve2, reject1, reject2; + var promise1 = new Promise(function (resolve, reject) { + resolve1 = resolve; + reject1 = reject; + }); + var promise2 = new Promise(function (resolve, reject) { + resolve2 = resolve; + reject2 = reject; + }); + + Zotero.DB.executeTransaction(function* () { + yield Zotero.Promise.delay(100); + assert.equal(Zotero.DB._asyncTransactionNestingLevel, 0); + yield Zotero.DB.queryAsync("INSERT INTO " + tmpTable + " VALUES (2)"); + // Make sure we're still in a transaction + Zotero.DB.transactionDate; + }) + .then(resolve1) + .catch(reject1); + + Zotero.DB.executeTransaction(function* () { + assert.equal(Zotero.DB._asyncTransactionNestingLevel, 1); + yield Zotero.DB.queryAsync("INSERT INTO " + tmpTable + " VALUES (1)"); + // Make sure we're still in a transaction + Zotero.DB.transactionDate; + }) + .then(resolve2) + .catch(reject2); + + yield Zotero.Promise.all([promise1, promise2]); + })); + it("should roll back on error", function* () { var tmpTable = "tmpRollbackOnError"; yield Zotero.DB.queryAsync("CREATE TABLE " + tmpTable + " (foo INT)");