From 034f667869521c4aee1268a5e30275233facdaaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Thu, 7 May 2026 12:58:18 -0700 Subject: [PATCH] Fix per-thread txn leaks when SafeNdbTxn init fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This addresses one failure path where if the value getter for a new transaction returns `nil`, it would leak a read transaction, causing it to remain open indefinitely, and leading to runaway storage usage. Changelog-Fixed: Fixed database transaction leak that could lead to higher storage usage Signed-off-by: Daniel D’Aquino --- nostrdb/NdbTxn.swift | 17 ++++++++- nostrdb/Test/NdbTests.swift | 70 +++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/nostrdb/NdbTxn.swift b/nostrdb/NdbTxn.swift index 6141f111..133cc71f 100644 --- a/nostrdb/NdbTxn.swift +++ b/nostrdb/NdbTxn.swift @@ -210,7 +210,22 @@ class SafeNdbTxn { print("txn: open gen\(generation) '\(name)' \(txn_count)") #endif let placeholderTxn = PlaceholderNdbTxn(txn: txn) - guard let val = valueGetter(placeholderTxn) else { return nil } + guard let val = valueGetter(placeholderTxn) else { + if inherited { + if let refCount = Thread.current.threadDictionary["ndb_txn_ref_count"] as? Int { + Thread.current.threadDictionary["ndb_txn_ref_count"] = max(0, refCount - 1) + } + return nil + } + + _ = try? ndb.withNdb({ + ndb_end_query(&placeholderTxn.txn) + }, maxWaitTimeout: .milliseconds(200)) + Thread.current.threadDictionary.removeObject(forKey: "ndb_txn") + Thread.current.threadDictionary.removeObject(forKey: "ndb_txn_ref_count") + Thread.current.threadDictionary.removeObject(forKey: "txn_generation") + return nil + } return SafeNdbTxn(ndb: ndb, txn: txn, val: val, generation: generation, inherited: inherited, name: name) } diff --git a/nostrdb/Test/NdbTests.swift b/nostrdb/Test/NdbTests.swift index 47426b00..41a6075f 100644 --- a/nostrdb/Test/NdbTests.swift +++ b/nostrdb/Test/NdbTests.swift @@ -22,6 +22,36 @@ func test_ndb_dir() -> String? { final class NdbTests: XCTestCase { var db_dir: String = "" + /// Saves and clears thread-local transaction state for deterministic transaction tests, then returns a restoration closure. + @discardableResult + private func resetThreadLocalTransactionState() -> () -> Void { + let threadDictionary = Thread.current.threadDictionary + let originalTxn = threadDictionary["ndb_txn"] + let originalRefCount = threadDictionary["ndb_txn_ref_count"] + let originalGeneration = threadDictionary["txn_generation"] + threadDictionary.removeObject(forKey: "ndb_txn") + threadDictionary.removeObject(forKey: "ndb_txn_ref_count") + threadDictionary.removeObject(forKey: "txn_generation") + + return { + if let originalTxn { + threadDictionary["ndb_txn"] = originalTxn + } else { + threadDictionary.removeObject(forKey: "ndb_txn") + } + if let originalRefCount { + threadDictionary["ndb_txn_ref_count"] = originalRefCount + } else { + threadDictionary.removeObject(forKey: "ndb_txn_ref_count") + } + if let originalGeneration { + threadDictionary["txn_generation"] = originalGeneration + } else { + threadDictionary.removeObject(forKey: "txn_generation") + } + } + } + override func setUpWithError() throws { guard let db = test_ndb_dir() else { XCTFail("Could not create temp directory") @@ -183,6 +213,46 @@ final class NdbTests: XCTestCase { let ndb_txn = Thread.current.threadDictionary.value(forKey: "ndb_txn") XCTAssertNil(ndb_txn) } + + /// Verifies that a failed top-level SafeNdbTxn creation fully cleans up thread-local transaction state. + func testSafeNdbTxnFailure_cleansUpTopLevelThreadLocalTransactionState() throws { + let restoreThreadLocalTransactionState = resetThreadLocalTransactionState() + defer { restoreThreadLocalTransactionState() } + + let ndb = try XCTUnwrap(Ndb(path: db_dir)) + + let txn = SafeNdbTxn.new(on: ndb, with: { _ in nil }, name: "failing_top_level_txn") + + XCTAssertNil(txn) + XCTAssertNil(Thread.current.threadDictionary["ndb_txn"], "Top-level SafeNdbTxn failure should clear the thread-local transaction") + XCTAssertNil(Thread.current.threadDictionary["ndb_txn_ref_count"], "Top-level SafeNdbTxn failure should clear the thread-local reference count") + XCTAssertNil(Thread.current.threadDictionary["txn_generation"], "Top-level SafeNdbTxn failure should clear the thread-local generation") + } + + /// Verifies that a failed inherited SafeNdbTxn creation decrements the ref-count without clearing the parent transaction. + func testSafeNdbTxnFailure_onInheritedTransactionRestoresParentThreadLocalState() throws { + let restoreThreadLocalTransactionState = resetThreadLocalTransactionState() + defer { restoreThreadLocalTransactionState() } + + let ndb = try XCTUnwrap(Ndb(path: db_dir)) + + let parent = try XCTUnwrap(SafeNdbTxn.new(on: ndb, with: { _ in 1 }, name: "parent_txn")) + let parentThreadTxn = Thread.current.threadDictionary["ndb_txn"] as? ndb_txn + XCTAssertNotNil(parentThreadTxn) + XCTAssertEqual(Thread.current.threadDictionary["ndb_txn_ref_count"] as? Int, 1) + + let child = SafeNdbTxn.new(on: ndb, with: { _ in nil }, name: "failing_child_txn") + + XCTAssertNil(child) + let restoredThreadTxn = Thread.current.threadDictionary["ndb_txn"] as? ndb_txn + XCTAssertNotNil(restoredThreadTxn, "Inherited SafeNdbTxn failure should keep the parent transaction installed") + XCTAssertEqual(restoredThreadTxn?.lmdb, parentThreadTxn?.lmdb) + XCTAssertEqual(restoredThreadTxn?.mdb_txn, parentThreadTxn?.mdb_txn) + XCTAssertEqual(Thread.current.threadDictionary["ndb_txn_ref_count"] as? Int, 1, "Inherited SafeNdbTxn failure should restore the parent ref-count") + XCTAssertEqual(Thread.current.threadDictionary["txn_generation"] as? Int, ndb.generation) + + _ = parent + } func test_decode_perf() throws { // This is an example of a performance test case.