From fd740e5612d875b3971029c6c1151a8d2373a54d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20G=C3=BCdelh=C3=B6fer?= <hannes@guedelhoefer.de>
Date: Thu, 16 May 2024 10:22:59 +0200
Subject: [PATCH 1/2] Add test for OOM case while closing transaction
 concurrently to running queries

I noticed that while closing a transaction and concurrently still having queries left running the driver eventually causes an OOM node error. This test case causes this exact error.
A fix will (hopefully) come in the next commit
---
 .../neo4j-driver/test/transaction.test.js     | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

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)
 })

From dc7b6a7299d165ff9785418a287ee30ad5f8962f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hannes=20G=C3=BCdelh=C3=B6fer?= <hannes@guedelhoefer.de>
Date: Thu, 16 May 2024 10:46:29 +0200
Subject: [PATCH 2/2] Fix running into an infinite loop when closing a
 transaction with still running queries.

When closing a transaction, that still had queries open the `_onErrorCallback` in core/src/transaction.ts:303 called
`resultStreamObserver.onError` for each result. The default behaviour for `FailedObvserver` (one implementation) is to call `this._beforeError`(core/src/internal/observer.ts:179), which is set to `onError`, which happens to be set to the mentioned
`_onErrorCallback`.
This results in an infinite async-loop, which gradualy consumes all available memory and crashes the process.

Because the `FAILED` state is only set in `_onErrorCallback` we can use it as a flag, to cut off the infinite recursion, when we are allready in the `FAILED` state.
---
 packages/core/src/transaction.ts                   | 6 ++++++
 packages/neo4j-driver-deno/lib/core/transaction.ts | 6 ++++++
 2 files changed, 12 insertions(+)

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 => {