Skip to content

Commit b238051

Browse files
danieldaquinodanieldaquino
committed
Add sync mechanism to prevent background crashes and fix ndb reopen order
This adds a sync mechanism in Ndb.swift to coordinate certain usage of nostrdb.c calls and the need to close nostrdb due to app lifecycle requirements. Furthermore, it fixes the order of operations when re-opening NostrDB, to avoid race conditions where a query uses an older Ndb generation. This sync mechanism allows multiple queries to happen simultaneously (from the Swift-side), while preventing ndb from simultaneously closing during such usages. It also does that while keeping the Ndb interface sync and nonisolated, which keeps the API easy to use from Swift/SwiftUI and allows for parallel operations to occur. If Swift Actors were to be used (e.g. creating an NdbActor), the Ndb.swift interface would change in such a way that it would propagate the need for several changes throughout the codebase, including loading logic in some ViewModels. Furthermore, it would likely decrease performance by forcing Ndb.swift operations to run sequentially when they could run in parallel. A changelog is not needed since the background crashes were not widely spread in the last public release. Changelog-None Closes: #3245 Signed-off-by: Daniel D’Aquino <[email protected]>
1 parent 8c578ce commit b238051

File tree

4 files changed

+273
-43
lines changed

4 files changed

+273
-43
lines changed

‎damus.xcodeproj/project.pbxproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1668,6 +1668,10 @@
16681668
D74EC8522E1856B70091DC51 /* NonCopyableLinkedList.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EC84E2E1856AF0091DC51 /* NonCopyableLinkedList.swift */; };
16691669
D74F430A2B23F0BE00425B75 /* DamusPurple.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74F43092B23F0BE00425B75 /* DamusPurple.swift */; };
16701670
D74F430C2B23FB9B00425B75 /* StoreObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74F430B2B23FB9B00425B75 /* StoreObserver.swift */; };
1671+
D75154BF2EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */; };
1672+
D75154C02EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */; };
1673+
D75154C12EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */; };
1674+
D75154C22EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */ = {isa = PBXBuildFile; fileRef = D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */; };
16711675
D753CEAA2BE9DE04001C3A5D /* MutingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D753CEA92BE9DE04001C3A5D /* MutingTests.swift */; };
16721676
D755B28D2D3E7D8800BBEEFA /* NIP37Draft.swift in Sources */ = {isa = PBXBuildFile; fileRef = D755B28C2D3E7D7D00BBEEFA /* NIP37Draft.swift */; };
16731677
D755B28E2D3E7D8800BBEEFA /* NIP37Draft.swift in Sources */ = {isa = PBXBuildFile; fileRef = D755B28C2D3E7D7D00BBEEFA /* NIP37Draft.swift */; };
@@ -2767,6 +2771,7 @@
27672771
D74EC84E2E1856AF0091DC51 /* NonCopyableLinkedList.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NonCopyableLinkedList.swift; sourceTree = "<group>"; };
27682772
D74F43092B23F0BE00425B75 /* DamusPurple.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurple.swift; sourceTree = "<group>"; };
27692773
D74F430B2B23FB9B00425B75 /* StoreObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreObserver.swift; sourceTree = "<group>"; };
2774+
D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NdbUseLock.swift; sourceTree = "<group>"; };
27702775
D753CEA92BE9DE04001C3A5D /* MutingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MutingTests.swift; sourceTree = "<group>"; };
27712776
D755B28C2D3E7D7D00BBEEFA /* NIP37Draft.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NIP37Draft.swift; sourceTree = "<group>"; };
27722777
D76556D52B1E6C08001B0CCC /* DamusPurpleWelcomeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurpleWelcomeView.swift; sourceTree = "<group>"; };
@@ -3299,6 +3304,7 @@
32993304
4C9054862A6AEB4500811EEC /* nostrdb */ = {
33003305
isa = PBXGroup;
33013306
children = (
3307+
D75154BE2EC5910600BF2CB2 /* NdbUseLock.swift */,
33023308
D74EC84E2E1856AF0091DC51 /* NonCopyableLinkedList.swift */,
33033309
D733F9E42D92C75C00317B11 /* UnownedNdbNote.swift */,
33043310
D7F5630F2DEE71BB008509DE /* NdbFilter.swift */,
@@ -5740,6 +5746,7 @@
57405746
4CC6AA792CAB688500989CEF /* sha256.c in Sources */,
57415747
4CC6AA7B2CAB688500989CEF /* likely.c in Sources */,
57425748
4CC6AA7F2CAB688500989CEF /* htable.c in Sources */,
5749+
D75154C02EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */,
57435750
4CC6AA862CAB688500989CEF /* list.c in Sources */,
57445751
4CC6AA872CAB688500989CEF /* utf8.c in Sources */,
57455752
4CC6AA892CAB688500989CEF /* debug.c in Sources */,
@@ -6781,6 +6788,7 @@
67816788
82D6FC3A2CD99F7900C925F4 /* WideEventView.swift in Sources */,
67826789
82D6FC3B2CD99F7900C925F4 /* LongformView.swift in Sources */,
67836790
82D6FC3C2CD99F7900C925F4 /* LongformPreview.swift in Sources */,
6791+
D75154C22EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */,
67846792
82D6FC3D2CD99F7900C925F4 /* EventShell.swift in Sources */,
67856793
82D6FC3E2CD99F7900C925F4 /* MentionView.swift in Sources */,
67866794
82D6FC3F2CD99F7900C925F4 /* EventLoaderView.swift in Sources */,
@@ -7206,6 +7214,7 @@
72067214
D73E5F302C6A97F4007EB227 /* EventProfile.swift in Sources */,
72077215
D73E5F312C6A97F4007EB227 /* EventMenu.swift in Sources */,
72087216
D73E5F322C6A97F4007EB227 /* EventMutingContainerView.swift in Sources */,
7217+
D75154C12EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */,
72097218
D73E5F332C6A97F4007EB227 /* ZapEvent.swift in Sources */,
72107219
5C8F97362EB46145009399B1 /* LiveStreamView.swift in Sources */,
72117220
D73E5F342C6A97F4007EB227 /* TextEvent.swift in Sources */,
@@ -7470,6 +7479,7 @@
74707479
4CC6AAC52CAB688500989CEF /* likely.c in Sources */,
74717480
4CC6AAC92CAB688500989CEF /* htable.c in Sources */,
74727481
4CC6AAD02CAB688500989CEF /* list.c in Sources */,
7482+
D75154BF2EC5910A00BF2CB2 /* NdbUseLock.swift in Sources */,
74737483
4CC6AAD12CAB688500989CEF /* utf8.c in Sources */,
74747484
4CC6AAD32CAB688500989CEF /* debug.c in Sources */,
74757485
4CC6AAD42CAB688500989CEF /* str.c in Sources */,

‎nostrdb/Ndb.swift‎

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import Foundation
99
import OSLog
10+
import Synchronization
1011

1112
fileprivate let APPLICATION_GROUP_IDENTIFIER = "group.com.damus"
1213

@@ -34,6 +35,7 @@ class Ndb {
3435
var generation: Int
3536
private var closed: Bool
3637
private var callbackHandler: Ndb.CallbackHandler
38+
let ndbAccessLock: Ndb.UseLockProtocol = initLock()
3739

3840
private static let DEFAULT_WRITER_SCRATCH_SIZE: Int32 = 2097152; // 2mb scratch size for the writer thread, it should match with the one specified in nostrdb.c
3941

@@ -158,6 +160,7 @@ class Ndb {
158160
self.ndb = db
159161
self.closed = false
160162
self.callbackHandler = callbackHandler
163+
self.ndbAccessLock.markNdbOpen()
161164
}
162165

163166
private static func migrate_db_location_if_needed() throws {
@@ -207,6 +210,12 @@ class Ndb {
207210
self.callbackHandler = Ndb.CallbackHandler()
208211
}
209212

213+
214+
// MARK: - Syncing use of the `ndb` C object
215+
// This is done to prevent race conditions where one thread is using ndb (e.g. querying), while another thread is closing ndb.
216+
217+
218+
210219
/// Mark NostrDB as "closed" without actually closing it.
211220
/// Useful when shutting down tasks that use NostrDB while avoiding new tasks from using it.
212221
func markClosed() {
@@ -216,10 +225,13 @@ class Ndb {
216225
func close() {
217226
guard !self.is_closed else { return }
218227
self.closed = true
219-
print("txn: CLOSING NOSTRDB")
220-
ndb_destroy(self.ndb.ndb)
221-
self.generation += 1
222-
print("txn: NOSTRDB CLOSED")
228+
try! self.ndbAccessLock.waitUntilNdbCanClose(thenClose: {
229+
print("txn: CLOSING NOSTRDB")
230+
ndb_destroy(self.ndb.ndb)
231+
self.generation += 1
232+
print("txn: NOSTRDB CLOSED")
233+
return false
234+
}, maxTimeout: .milliseconds(2000))
223235
}
224236

225237
func reopen() -> Bool {
@@ -229,9 +241,10 @@ class Ndb {
229241
}
230242

231243
print("txn: NOSTRDB REOPENED (gen \(generation))")
232-
244+
245+
self.ndb = db // Set the new DB before marking it as open to prevent access to the old DB
233246
self.closed = false
234-
self.ndb = db
247+
self.ndbAccessLock.markNdbOpen()
235248
return true
236249
}
237250

@@ -628,32 +641,35 @@ class Ndb {
628641
/// - maxResults: Maximum number of results to return
629642
/// - Returns: Array of note keys matching the filters
630643
/// - Throws: NdbStreamError if the query fails
631-
func query<Y>(with txn: NdbTxn<Y>, filters: [NdbFilter], maxResults: Int) throws(NdbStreamError) -> [NoteKey] {
632-
guard !self.is_closed else { throw .ndbClosed }
633-
let filtersPointer = UnsafeMutablePointer<ndb_filter>.allocate(capacity: filters.count)
634-
defer { filtersPointer.deallocate() }
635-
636-
for (index, ndbFilter) in filters.enumerated() {
637-
filtersPointer.advanced(by: index).pointee = ndbFilter.ndbFilter
638-
}
639-
640-
let count = UnsafeMutablePointer<Int32>.allocate(capacity: 1)
641-
defer { count.deallocate() }
642-
643-
let results = UnsafeMutablePointer<ndb_query_result>.allocate(capacity: maxResults)
644-
defer { results.deallocate() }
645-
646-
guard !self.is_closed else { throw .ndbClosed }
647-
guard ndb_query(&txn.txn, filtersPointer, Int32(filters.count), results, Int32(maxResults), count) == 1 else {
648-
throw NdbStreamError.initialQueryFailed
649-
}
650-
651-
var noteIds: [NoteKey] = []
652-
for i in 0..<count.pointee {
653-
noteIds.append(results.advanced(by: Int(i)).pointee.note_id)
654-
}
655-
656-
return noteIds
644+
func query<Y>(with txn: NdbTxn<Y>, filters: [NdbFilter], maxResults: Int) throws -> [NoteKey] {
645+
guard !self.is_closed else { throw NdbStreamError.ndbClosed }
646+
return try self.ndbAccessLock.keepNdbOpen(during: {
647+
guard txn.generation == self.generation else { throw NdbStreamError.cannotOpenTransaction }
648+
649+
let filtersPointer = UnsafeMutablePointer<ndb_filter>.allocate(capacity: filters.count)
650+
defer { filtersPointer.deallocate() }
651+
652+
for (index, ndbFilter) in filters.enumerated() {
653+
filtersPointer.advanced(by: index).pointee = ndbFilter.ndbFilter
654+
}
655+
656+
let count = UnsafeMutablePointer<Int32>.allocate(capacity: 1)
657+
defer { count.deallocate() }
658+
659+
let results = UnsafeMutablePointer<ndb_query_result>.allocate(capacity: maxResults)
660+
defer { results.deallocate() }
661+
662+
guard ndb_query(&txn.txn, filtersPointer, Int32(filters.count), results, Int32(maxResults), count) == 1 else {
663+
throw NdbStreamError.initialQueryFailed
664+
}
665+
666+
var noteIds: [NoteKey] = []
667+
for i in 0..<count.pointee {
668+
noteIds.append(results.advanced(by: Int(i)).pointee.note_id)
669+
}
670+
671+
return noteIds
672+
}, maxWaitTimeout: .milliseconds(500))
657673
}
658674

659675
/// Safe wrapper around `ndb_subscribe` that handles all pointer management
@@ -1002,4 +1018,3 @@ func getDebugCheckedRoot<T: FlatBufferObject>(byteBuffer: inout ByteBuffer) thro
10021018
func remove_file_prefix(_ str: String) -> String {
10031019
return str.replacingOccurrences(of: "file://", with: "")
10041020
}
1005-

‎nostrdb/NdbTxn.swift‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,16 +43,18 @@ class NdbTxn<T>: RawNdbTxnAccessible {
4343
let new_ref_count = ref_count + 1
4444
Thread.current.threadDictionary["ndb_txn_ref_count"] = new_ref_count
4545
} else {
46-
self.txn = ndb_txn()
4746
guard !ndb.is_closed else { return nil }
48-
self.generation = ndb.generation
49-
#if TXNDEBUG
50-
txn_count += 1
51-
#endif
52-
let ok = ndb_begin_query(ndb.ndb.ndb, &self.txn) != 0
53-
if !ok {
54-
return nil
55-
}
47+
let txn: ndb_txn? = try? ndb.ndbAccessLock.keepNdbOpen(during: {
48+
var txn = ndb_txn()
49+
#if TXNDEBUG
50+
txn_count += 1
51+
#endif
52+
let ok = ndb_begin_query(ndb.ndb.ndb, &txn) != 0
53+
guard ok else { return nil }
54+
return txn
55+
}, maxWaitTimeout: .milliseconds(200))
56+
guard let txn else { return nil }
57+
self.txn = txn
5658
self.generation = ndb.generation
5759
Thread.current.threadDictionary["ndb_txn"] = self.txn
5860
Thread.current.threadDictionary["ndb_txn_ref_count"] = 1
@@ -97,7 +99,13 @@ class NdbTxn<T>: RawNdbTxnAccessible {
9799
Thread.current.threadDictionary["ndb_txn_ref_count"] = new_ref_count
98100
assert(new_ref_count >= 0, "NdbTxn reference count should never be below zero")
99101
if new_ref_count <= 0 {
100-
ndb_end_query(&self.txn)
102+
guard !ndb.is_closed else {
103+
print("txn: not closing. db closed")
104+
return
105+
}
106+
_ = try? ndb.ndbAccessLock.keepNdbOpen(during: {
107+
ndb_end_query(&self.txn)
108+
}, maxWaitTimeout: .milliseconds(200))
101109
Thread.current.threadDictionary.removeObject(forKey: "ndb_txn")
102110
Thread.current.threadDictionary.removeObject(forKey: "ndb_txn_ref_count")
103111
}

0 commit comments

Comments
 (0)