Merge pull request #3739 from danieldaquino/gh-3738

Fix SafeNdbTxn failure cleanup and reclaim stale LMDB readers on startup

nostrdb/NdbTxn.swift

@@ -210,7 +210,22 @@ class SafeNdbTxn<T: ~Copyable> {
         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<T>(ndb: ndb, txn: txn, val: val, generation: generation, inherited: inherited, name: name)
     }
 

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")
@@ -184,6 +214,46 @@ final class NdbTests: XCTestCase {
         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<Int>.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<Int>.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<Int>.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.
         self.measure {

nostrdb/src/nostrdb.c

@@ -165,6 +165,32 @@ struct ndb_lmdb {
 	MDB_dbi dbs[NDB_DBS];
 };
 
+/**
+ * Clears stale LMDB reader slots after opening the environment.
+ *
+ * This protects startup on platforms where reader slots are not reclaimed
+ * automatically after process termination.
+ *
+ * @param[in] env The LMDB environment to inspect.
+ * @return 1 when the check succeeds, 0 when LMDB reports an error.
+ */
+static int ndb_lmdb_reader_check(MDB_env *env)
+{
+	int rc;
+	int dead = 0;
+
+	rc = mdb_reader_check(env, &dead);
+	if (rc != MDB_SUCCESS) {
+		fprintf(stderr, "mdb_reader_check failed: %s\n", mdb_strerror(rc));
+		return 0;
+	}
+
+	if (dead > 0)
+		fprintf(stderr, "mdb_reader_check cleared %d stale reader(s)\n", dead);
+
+	return 1;
+}
+
 struct ndb_writer {
 	struct ndb_lmdb *lmdb;
 	struct ndb_monitor *monitor;
@@ -5849,6 +5875,12 @@ static int ndb_init_lmdb(const char *filename, struct ndb_lmdb *lmdb, size_t map
 		return 0;
 	}
 
+	if (!ndb_lmdb_reader_check(lmdb->env)) {
+		mdb_env_close(lmdb->env);
+		lmdb->env = NULL;
+		return 0;
+	}
+
 	// Initialize DBs
 	if ((rc = mdb_txn_begin(lmdb->env, NULL, 0, &txn))) {
 		fprintf(stderr, "mdb_txn_begin failed, error %d\n", rc);