diff --git a/packages/core/src/transaction.ts b/packages/core/src/transaction.ts index b3767c6b3..1f3dee59d 100644 --- a/packages/core/src/transaction.ts +++ b/packages/core/src/transaction.ts @@ -299,6 +299,12 @@ class Transaction { // error will be "acknowledged" by sending a RESET message // database will then forget about this transaction and cleanup all corresponding resources // it is thus safe to move this transaction to a FAILED state and disallow any further interactions with it + + if (this._state === _states.FAILED) { + // already failed, nothing to do + // if we call onError for each result again, we might run into an infinite loop, that causes an OOM eventually + return Promise.resolve(null) + } this._state = _states.FAILED this._onClose() this._results.forEach(result => { diff --git a/packages/neo4j-driver-deno/lib/core/transaction.ts b/packages/neo4j-driver-deno/lib/core/transaction.ts index c0dd5ae96..6291bd961 100644 --- a/packages/neo4j-driver-deno/lib/core/transaction.ts +++ b/packages/neo4j-driver-deno/lib/core/transaction.ts @@ -299,6 +299,12 @@ class Transaction { // error will be "acknowledged" by sending a RESET message // database will then forget about this transaction and cleanup all corresponding resources // it is thus safe to move this transaction to a FAILED state and disallow any further interactions with it + + if (this._state === _states.FAILED) { + // already failed, nothing to do + // if we call onError for each result again, we might run into an infinite loop, that causes an OOM eventually + return Promise.resolve(null) + } this._state = _states.FAILED this._onClose() this._results.forEach(result => { diff --git a/packages/neo4j-driver/test/transaction.test.js b/packages/neo4j-driver/test/transaction.test.js index 40c088c9b..f7b998ea8 100644 --- a/packages/neo4j-driver/test/transaction.test.js +++ b/packages/neo4j-driver/test/transaction.test.js @@ -499,4 +499,29 @@ describe('#integration transaction', () => { .catch(done.fail.bind(done)) ) } + + /** + * This test is to verify that the driver does not run out of memory when one of the queries fails, + * while others are concurrently running and the transaction is being committed/closed. + * + * Because this causes an infinite loop in the failure case, the test has a timeout of 50 seconds, in order to actualy + * trip the OOM error, and not run into a timeout error prematurely. + */ + it('transactions should not run OOM when one of the queries fails', async () => { + const transaction = await session.beginTransaction() + const queries = Array.from( + { length: 10 }, + (_, i) => () => transaction.run(`RETURN ${i}`) + ) + const destroy = () => transaction.close() + + const calls = [...queries, destroy, ...queries] + const promises = calls.map(call => call()) + try { + await Promise.all(promises) + } catch (e) { + // This is the success case, as we god an exception, not run into an OOM + expect(e.message).toBe('Cannot run query in this transaction, because it has already been rolled back.') + } + }, 50000) })