tyiu/damus
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix profile crash

Browse Source 
This fixes a crash that would occasionally occur when visiting profiles.

NdbTxn objects were being deinitialized on different threads from their
initialization, causing incorrect reference count decrements in thread-local
transaction dictionaries. This led to premature destruction of shared ndb_txn
C objects still in use by other tasks, resulting in use-after-free crashes.

The root cause is that Swift does not guarantee tasks resume on the same
thread after await suspension points, while NdbTxn's init/deinit rely on
thread-local storage to track inherited transaction reference counts.

This means that `NdbTxn` objects cannot be used in async functions, as
that may cause the garbage collector to deinitialize `NdbTxn` at the end
of such function, which may be running on a different thread at that
point, causing the issue explained above.

The fix in this case is to eliminate the `async` version of the
`NdbNoteLender.borrow` method, and update usages to utilize other
available methods.

Note: This is a rewrite of the fix in https://github.com/damus-io/damus/pull/3329

Note 2: This relates to the fix of an unreleased feature, so therefore no
changelog is needed.

Changelog-None
Co-authored-by: alltheseas <64376233+alltheseas@users.noreply.github.com>
Closes: https://github.com/damus-io/damus/issues/3327
Signed-off-by: Daniel D’Aquino <daniel@daquino.me>
This commit is contained in:
Daniel D’Aquino
2025-11-19 20:14:34 -08:00
parent d651084465
commit 52115d07c2
4 changed files with 94 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
damus/Core/Networking/NostrNetworkManager/UserRelayListManager.swift
View File

@@ -135,13 +135,12 @@ extension NostrNetworkManager {
let filter = NostrFilter(kinds: [.relay_list], authors: [delegate.keypair.pubkey])
for await noteLender in self.reader.streamIndefinitely(filters: [filter]) {
let currentRelayListCreationDate = self.getUserCurrentRelayListCreationDate()
try? await noteLender.borrow({ note in
guard note.pubkey == self.delegate.keypair.pubkey else { return } // Ensure this new list was ours
guard note.createdAt > (currentRelayListCreationDate ?? 0) else { return } // Ensure this is a newer list
guard let relayList = try? NIP65.RelayList(event: note) else { return } // Ensure it is a valid NIP-65 list
guard let note = noteLender.justGetACopy() else { continue }
guard note.pubkey == self.delegate.keypair.pubkey else { continue } // Ensure this new list was ours
guard note.created_at > (currentRelayListCreationDate ?? 0) else { continue } // Ensure this is a newer list
guard let relayList = try? NIP65.RelayList(event: note) else { continue } // Ensure it is a valid NIP-65 list
try? await self.set(userRelayList: relayList) // Set the validated list
})
try? await self.set(userRelayList: relayList) // Set the validated list
}
}

16
damus/Features/Profile/Models/ProfileModel.swift
View File

@@ -116,15 +116,13 @@ class ProfileModel: ObservableObject, Equatable {
let conversations_filter_us = NostrFilter(kinds: conversation_kinds, pubkeys: [pubkey], limit: limit, authors: [damus.pubkey])
print("subscribing to conversation events from and to profile \(pubkey)")
for await noteLender in self.damus.nostrNetwork.reader.streamIndefinitely(filters: [conversations_filter_them, conversations_filter_us]) {
try? await noteLender.borrow { ev in
if await !seen_event.contains(ev.id) {
let event = ev.toOwned()
Task { await self.add_event(event) }
conversation_events.insert(ev.id)
}
else if !conversation_events.contains(ev.id) {
conversation_events.insert(ev.id)
}
guard let event = noteLender.justGetACopy() else { continue }
if await !seen_event.contains(event.id) {
Task { await self.add_event(event) }
conversation_events.insert(event.id)
}
else if !conversation_events.contains(event.id) {
conversation_events.insert(event.id)
}
}
}

81
damusTests/NostrNetworkManagerTests/NostrNetworkManagerTests.swift
View File

@@ -163,4 +163,85 @@ class NostrNetworkManagerTests: XCTestCase {
XCTAssertEqual(count, expectedCount, "Should receive all \(expectedCount) events")
XCTAssertEqual(receivedIds.count, expectedCount, "Should receive \(expectedCount) unique events")
}
/// Ensures the relay list listener ignores a bad event and still applies the next valid update.
func testRelayListListenerSkipsInvalidEventsAndContinues() async throws {
let ndb = Ndb.test
let delegate = MockNetworkDelegate(ndb: ndb, keypair: test_keypair, bootstrapRelays: [RelayURL("wss://relay.damus.io")!])
let pool = RelayPool(ndb: ndb, keypair: test_keypair)
let reader = MockSubscriptionManager(pool: pool, ndb: ndb)
let manager = SpyUserRelayListManager(delegate: delegate, pool: pool, reader: reader)
let appliedExpectation = expectation(description: "Applies valid relay list after encountering an invalid event")
manager.setExpectation = appliedExpectation
guard let invalidEvent = NostrEvent(content: "invalid", keypair: test_keypair, kind: NostrKind.metadata.rawValue, createdAt: 1) else {
XCTFail("Failed to create invalid test event")
return
}
let validRelayList = NIP65.RelayList(relays: [RelayURL("wss://relay-2.damus.io")!])
guard let validEvent = validRelayList.toNostrEvent(keypair: test_keypair_full) else {
XCTFail("Failed to create valid relay list event")
return
}
// Feed the listener a bad event followed by a valid relay list.
reader.queuedLenders = [.owned(invalidEvent), .owned(validEvent)]
await manager.listenAndHandleRelayUpdates()
await fulfillment(of: [appliedExpectation], timeout: 1.0)
XCTAssertEqual(manager.setCallCount, 1)
XCTAssertEqual(manager.appliedRelayLists.first?.relays.count, validRelayList.relays.count)
}
}
// MARK: - Test doubles
private final class MockNetworkDelegate: NostrNetworkManager.Delegate {
var ndb: Ndb
var keypair: Keypair
var latestRelayListEventIdHex: String?
var latestContactListEvent: NostrEvent?
var bootstrapRelays: [RelayURL]
var developerMode: Bool = false
var experimentalLocalRelayModelSupport: Bool = false
var relayModelCache: RelayModelCache
var relayFilters: RelayFilters
var nwcWallet: WalletConnectURL?
init(ndb: Ndb, keypair: Keypair, bootstrapRelays: [RelayURL]) {
self.ndb = ndb
self.keypair = keypair
self.bootstrapRelays = bootstrapRelays
self.relayModelCache = RelayModelCache()
self.relayFilters = RelayFilters(our_pubkey: keypair.pubkey)
}
}
private final class MockSubscriptionManager: NostrNetworkManager.SubscriptionManager {
var queuedLenders: [NdbNoteLender] = []
init(pool: RelayPool, ndb: Ndb) {
super.init(pool: pool, ndb: ndb, experimentalLocalRelayModelSupport: false)
}
override func streamIndefinitely(filters: [NostrFilter], to desiredRelays: [RelayURL]? = nil, streamMode: NostrNetworkManager.StreamMode? = nil, id: UUID? = nil) -> AsyncStream<NdbNoteLender> {
let lenders = queuedLenders
return AsyncStream { continuation in
lenders.forEach { continuation.yield($0) }
continuation.finish()
}
}
}
private final class SpyUserRelayListManager: NostrNetworkManager.UserRelayListManager {
var setCallCount = 0
var appliedRelayLists: [NIP65.RelayList] = []
var setExpectation: XCTestExpectation?
override func set(userRelayList: NIP65.RelayList) async throws(UpdateError) {
setCallCount += 1
appliedRelayLists.append(userRelayList)
setExpectation?.fulfill()
}
}

13
nostrdb/UnownedNdbNote.swift
View File

@@ -66,19 +66,6 @@ enum NdbNoteLender: Sendable {
}
}
/// Borrows the note temporarily (asynchronously)
func borrow<T>(_ lendingFunction: (_: borrowing UnownedNdbNote) async throws -> T) async throws -> T {
switch self {
case .ndbNoteKey(let ndb, let noteKey):
guard !ndb.is_closed else { throw LendingError.ndbClosed }
guard let ndbNoteTxn = ndb.lookup_note_by_key(noteKey) else { throw LendingError.errorLoadingNote }
guard let unownedNote = UnownedNdbNote(ndbNoteTxn) else { throw LendingError.errorLoadingNote }
return try await lendingFunction(unownedNote)
case .owned(let note):
return try await lendingFunction(UnownedNdbNote(note))
}
}
/// Gets an owned copy of the note
func getCopy() throws -> NdbNote {
return try self.borrow({ ev in