From b562b930cccc1e50f4916758f31ade0840314682 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= <daniel@daquino.me>
Date: Mon, 3 Nov 2025 15:47:45 -0800
Subject: [PATCH] Prevent new NostrDB streaming tasks from opening when NostrDB
 has begun to close
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have mechanisms in place to close NostrDB streams when the database needs to
close; however, there is a short time window where those streams are
closing down but the database still has its "open" property set to `true`,
which means that new NostrDB streams may open. If that happens, those
new streams will still be active when NostrDB gets closed down,
potentially causing memory crashes.

This was found by inspecting several crash logs and noticing that:
- most of the `ndb.close` calls are coming from the general
  backgrounding task (not the last resort backgrounding task),
  where all old tasks are guaranteed to have closed (we wait for all of
  them to close before proceeding to closing NostrDB).
- the stack traces of the crashed threads show that, in most cases, the
  stream crashes while they are in the query stage (which means that
  those must have been very recently opened).

The issue was mitigated by signalling that NostrDB has closed (without
actually closing it) before cancelling any streaming tasks and officially
closing NostrDB. This way, new NostrDB streaming tasks will notice that
the database is closed and will wait for it to reopen.

No changelog entry is needed as this issue was introduced after our last public
release.

Changelog-None
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
---
 damus/ContentView.swift                              |  9 ++-------
 .../NostrNetworkManager/NostrNetworkManager.swift    | 12 +++++++++++-
 nostrdb/Ndb.swift                                    |  6 ++++++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/damus/ContentView.swift b/damus/ContentView.swift
index bc602512..da6121ea 100644
--- a/damus/ContentView.swift
+++ b/damus/ContentView.swift
@@ -538,9 +538,7 @@ struct ContentView: View {
                     Log.debug("App background signal handling: App being backgrounded", for: .app_lifecycle)
                     let startTime = CFAbsoluteTimeGetCurrent()
                     await damus_state.nostrNetwork.handleAppBackgroundRequest()  // Close ndb streaming tasks before closing ndb to avoid memory errors
-                    Log.debug("App background signal handling: Nostr network closed after %.2f seconds", for: .app_lifecycle, CFAbsoluteTimeGetCurrent() - startTime)
-                    damus_state.ndb.close()
-                    Log.debug("App background signal handling: Ndb closed after %.2f seconds", for: .app_lifecycle, CFAbsoluteTimeGetCurrent() - startTime)
+                    Log.debug("App background signal handling: Nostr network and Ndb closed after %.2f seconds", for: .app_lifecycle, CFAbsoluteTimeGetCurrent() - startTime)
                     this_app.endBackgroundTask(bgTask)
                 }
                 break
@@ -552,9 +550,7 @@ struct ContentView: View {
                 Task {
                     await damusClosingTask?.value  // Wait for the closing task to finish before reopening things, to avoid race conditions
                     damusClosingTask = nil
-                    damus_state.ndb.reopen()
-                    // Pinging the network will automatically reconnect any dead websocket connections
-                    await damus_state.nostrNetwork.ping()
+                    await damus_state.nostrNetwork.handleAppForegroundRequest()
                 }
             @unknown default:
                 break
@@ -1144,7 +1140,6 @@ extension LossyLocalNotification {
     }
 }
 
-
 func logout(_ state: DamusState?)
 {
     state?.close()
diff --git a/damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift b/damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift
index 6bc1a17f..69b61fe8 100644
--- a/damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift
+++ b/damus/Core/Networking/NostrNetworkManager/NostrNetworkManager.swift
@@ -59,9 +59,19 @@ class NostrNetworkManager {
         await self.pool.disconnect()
     }
     
-    func handleAppBackgroundRequest() async {
+    func handleAppBackgroundRequest(beforeClosingNdb operationBeforeClosingNdb: (() async -> Void)? = nil) async {
+        // Mark NDB as closed without actually closing it, to avoid new tasks from using NostrDB
+        // self.delegate.ndb.markClosed()
         await self.reader.cancelAllTasks()
         await self.pool.cleanQueuedRequestForSessionEnd()
+        await operationBeforeClosingNdb?()
+        self.delegate.ndb.close()
+    }
+    
+    func handleAppForegroundRequest() async {
+        self.delegate.ndb.reopen()
+        // Pinging the network will automatically reconnect any dead websocket connections
+        await self.ping()
     }
     
     func close() async {
diff --git a/nostrdb/Ndb.swift b/nostrdb/Ndb.swift
index b171559d..97347658 100644
--- a/nostrdb/Ndb.swift
+++ b/nostrdb/Ndb.swift
@@ -207,6 +207,12 @@ class Ndb {
         self.callbackHandler = Ndb.CallbackHandler()
     }
     
+    /// Mark NostrDB as "closed" without actually closing it.
+    /// Useful when shutting down tasks that use NostrDB while avoiding new tasks from using it.
+    func markClosed() {
+        self.closed = true
+    }
+    
     func close() {
         guard !self.is_closed else { return }
         self.closed = true