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

txn: fix subtle transaction inheritence bugs

Browse Source 
This fixes subtle bugs with transaction inheritence. Since we were not
passing the inherited state to moved value, we were sometimes committing
transactions more than once.

Changelog-Fixed: Fix many nostrdb transaction related crashes
This commit is contained in:
William Casarin
2024-01-26 14:03:49 -08:00
parent f190b6414c
commit e3642b92d1
8 changed files with 53 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
damus/Models/NoteContent.swift
View File

@@ -183,7 +183,7 @@ func attributed_string_attach_icon(_ astr: inout AttributedString, img: UIImage)
} }
func getDisplayName(pk: Pubkey, profiles: Profiles) -> String { func getDisplayName(pk: Pubkey, profiles: Profiles) -> String {
let profile_txn = profiles.lookup(id: pk) let profile_txn = profiles.lookup(id: pk, txn_name: "getDisplayName")
let profile = profile_txn?.unsafeUnownedValue let profile = profile_txn?.unsafeUnownedValue
return Profile.displayName(profile: profile, pubkey: pk).username.truncate(maxLength: 50) return Profile.displayName(profile: profile, pubkey: pk).username.truncate(maxLength: 50)
} }

2
damus/Models/NotificationsManager.swift
View File

@@ -77,7 +77,7 @@ func generate_local_notification_object(from ev: NostrEvent, state: HeadlessDamu
} else if type == .like, } else if type == .like,
state.settings.like_notification, state.settings.like_notification,
let evid = ev.referenced_ids.last, let evid = ev.referenced_ids.last,
let txn = state.ndb.lookup_note(evid), let txn = state.ndb.lookup_note(evid, txn_name: "local_notification_like"),
let liked_event = txn.unsafeUnownedValue?.to_owned() let liked_event = txn.unsafeUnownedValue?.to_owned()
{ {
let content_preview = render_notification_content_preview(ev: liked_event, profiles: state.profiles, keypair: state.keypair) let content_preview = render_notification_content_preview(ev: liked_event, profiles: state.profiles, keypair: state.keypair)

4
damus/Nostr/Profiles.swift
View File

@@ -85,8 +85,8 @@ class Profiles {
ndb.search_profile(query, limit: limit, txn: txn) ndb.search_profile(query, limit: limit, txn: txn)
} }
func lookup(id: Pubkey) -> NdbTxn<Profile?>? { func lookup(id: Pubkey, txn_name: String? = nil) -> NdbTxn<Profile?>? {
guard let txn = ndb.lookup_profile(id) else { guard let txn = ndb.lookup_profile(id, txn_name: txn_name) else {
return nil return nil
} }
return txn.map({ pr in pr?.profile }) return txn.map({ pr in pr?.profile })

2
damus/Views/Profile/ProfilePicView.swift
View File

@@ -126,7 +126,7 @@ struct ProfilePicView: View {
} }
func get_profile_url(picture: String?, pubkey: Pubkey, profiles: Profiles) -> URL { func get_profile_url(picture: String?, pubkey: Pubkey, profiles: Profiles) -> URL {
let pic = picture ?? profiles.lookup(id: pubkey)?.map({ $0?.picture }).value ?? robohash(pubkey) let pic = picture ?? profiles.lookup(id: pubkey, txn_name: "get_profile_url")?.map({ $0?.picture }).value ?? robohash(pubkey)
if let url = URL(string: pic) { if let url = URL(string: pic) {
return url return url
} }

7
damus/Views/QRCodeView.swift
View File

@@ -118,10 +118,11 @@ struct QRCodeView: View {
var QRView: some View { var QRView: some View {
VStack(alignment: .center) { VStack(alignment: .center) {
let profile_txn = damus_state.profiles.lookup(id: pubkey) let profile_txn = damus_state.profiles.lookup(id: pubkey, txn_name: "qrview-profile")
let profile = profile_txn?.unsafeUnownedValue let profile = profile_txn?.unsafeUnownedValue
let our_profile_txn = damus_state.profiles.lookup(id: damus_state.pubkey) let our_profile = profile_txn.flatMap({ ptxn in
let our_profile = our_profile_txn?.unsafeUnownedValue damus_state.ndb.lookup_profile_with_txn(damus_state.pubkey, txn: ptxn)?.profile
})
if our_profile?.picture != nil { if our_profile?.picture != nil {
ProfilePicView(pubkey: pubkey, size: 90.0, highlight: .custom(DamusColors.white, 3.0), profiles: damus_state.profiles, disable_animation: damus_state.settings.disable_animation) ProfilePicView(pubkey: pubkey, size: 90.0, highlight: .custom(DamusColors.white, 3.0), profiles: damus_state.profiles, disable_animation: damus_state.settings.disable_animation)

4
damus/Views/SideMenuView.swift
View File

@@ -89,8 +89,8 @@ struct SideMenuView: View {
} }
var TopProfile: some View { var TopProfile: some View {
let profile_txn = damus_state.profiles.lookup(id: damus_state.pubkey) let profile_txn = damus_state.ndb.lookup_profile(damus_state.pubkey, txn_name: "top_profile")
let profile = profile_txn?.unsafeUnownedValue let profile = profile_txn?.unsafeUnownedValue?.profile
return VStack(alignment: .leading, spacing: verticalSpacing) { return VStack(alignment: .leading, spacing: verticalSpacing) {
HStack { HStack {
ProfilePicView(pubkey: damus_state.pubkey, size: 60, highlight: .none, profiles: damus_state.profiles, disable_animation: damus_state.settings.disable_animation) ProfilePicView(pubkey: damus_state.pubkey, size: 60, highlight: .none, profiles: damus_state.profiles, disable_animation: damus_state.settings.disable_animation)

28
nostrdb/Ndb.swift
View File

@@ -76,7 +76,8 @@ class Ndb {
static private var db_files: [String] = ["data.mdb", "lock.mdb"] static private var db_files: [String] = ["data.mdb", "lock.mdb"]
static var empty: Ndb { static var empty: Ndb {
Ndb(ndb: ndb_t(ndb: nil)) print("txn: NOSTRDB EMPTY")
return Ndb(ndb: ndb_t(ndb: nil))
} }
static func open(path: String? = nil, owns_db_file: Bool = true) -> ndb_t? { static func open(path: String? = nil, owns_db_file: Bool = true) -> ndb_t? {
@@ -187,6 +188,7 @@ class Ndb {
self.closed = true self.closed = true
print("txn: CLOSING NOSTRDB") print("txn: CLOSING NOSTRDB")
ndb_destroy(self.ndb.ndb) ndb_destroy(self.ndb.ndb)
self.generation += 1
print("txn: NOSTRDB CLOSED") print("txn: NOSTRDB CLOSED")
} }
@@ -195,8 +197,9 @@ class Ndb {
let db = Self.open(path: self.path, owns_db_file: self.owns_db) else { let db = Self.open(path: self.path, owns_db_file: self.owns_db) else {
return false return false
} }
print("txn: NOSTRDB REOPENED (gen \(generation))")
self.generation += 1
self.closed = false self.closed = false
self.ndb = db self.ndb = db
return true return true
@@ -368,14 +371,14 @@ class Ndb {
return txn.value return txn.value
} }
func lookup_note(_ id: NoteId) -> NdbTxn<NdbNote?>? { func lookup_note(_ id: NoteId, txn_name: String? = nil) -> NdbTxn<NdbNote?>? {
NdbTxn(ndb: self) { txn in NdbTxn(ndb: self, name: txn_name) { txn in
lookup_note_with_txn_inner(id: id, txn: txn) lookup_note_with_txn_inner(id: id, txn: txn)
} }
} }
func lookup_profile(_ pubkey: Pubkey) -> NdbTxn<ProfileRecord?>? { func lookup_profile(_ pubkey: Pubkey, txn_name: String? = nil) -> NdbTxn<ProfileRecord?>? {
NdbTxn(ndb: self) { txn in NdbTxn(ndb: self, name: txn_name) { txn in
lookup_profile_with_txn_inner(pubkey: pubkey, txn: txn) lookup_profile_with_txn_inner(pubkey: pubkey, txn: txn)
} }
} }
@@ -456,17 +459,8 @@ class Ndb {
} }
deinit { deinit {
//print("txn: Ndb de-init close") print("txn: Ndb de-init")
// self.close()
// DO NOT CLOSE IN DESTRUCTOR! Destructor deinit is clearly
// not reliable, since it seems to be getting called more than
// once when we have a few references to the database.
//
// Not sure if this is indicitable of a larger problem but I've
// experienced nothing but hell when relying on de-init for
// global resources. Free them manually.
//
// EVIL --> self.close() <-- EVIL
} }
} }

46
nostrdb/NdbTxn.swift
View File

@@ -19,17 +19,16 @@ class NdbTxn<T> {
var inherited: Bool var inherited: Bool
var ndb: Ndb var ndb: Ndb
var generation: Int var generation: Int
var name: String
init?(ndb: Ndb, with: (NdbTxn<T>) -> T = { _ in () }) { init?(ndb: Ndb, with: (NdbTxn<T>) -> T = { _ in () }, name: String? = nil) {
guard !ndb.closed else { return nil } guard !ndb.is_closed else { return nil }
self.name = name ?? "txn"
self.ndb = ndb self.ndb = ndb
self.generation = ndb.generation self.generation = ndb.generation
#if TXNDEBUG
txn_count += 1
print("opening transaction \(txn_count)")
#endif
if let active_txn = Thread.current.threadDictionary["ndb_txn"] as? ndb_txn { if let active_txn = Thread.current.threadDictionary["ndb_txn"] as? ndb_txn {
// some parent thread is active, use that instead // some parent thread is active, use that instead
print("txn: inherited txn")
self.txn = active_txn self.txn = active_txn
self.inherited = true self.inherited = true
self.generation = Thread.current.threadDictionary["txn_generation"] as! Int self.generation = Thread.current.threadDictionary["txn_generation"] as! Int
@@ -37,6 +36,9 @@ class NdbTxn<T> {
self.txn = ndb_txn() self.txn = ndb_txn()
guard !ndb.is_closed else { return nil } guard !ndb.is_closed else { return nil }
self.generation = ndb.generation self.generation = ndb.generation
#if TXNDEBUG
txn_count += 1
#endif
let ok = ndb_begin_query(ndb.ndb.ndb, &self.txn) != 0 let ok = ndb_begin_query(ndb.ndb.ndb, &self.txn) != 0
if !ok { if !ok {
return nil return nil
@@ -46,17 +48,21 @@ class NdbTxn<T> {
Thread.current.threadDictionary["txn_generation"] = ndb.generation Thread.current.threadDictionary["txn_generation"] = ndb.generation
self.inherited = false self.inherited = false
} }
#if TXNDEBUG
print("txn: open gen\(self.generation) '\(self.name)' \(txn_count)")
#endif
self.moved = false self.moved = false
self.val = with(self) self.val = with(self)
} }
private init(ndb: Ndb, txn: ndb_txn, val: T) { private init(ndb: Ndb, txn: ndb_txn, val: T, generation: Int, inherited: Bool, name: String) {
self.txn = txn self.txn = txn
self.val = val self.val = val
self.moved = false self.moved = false
self.inherited = false self.inherited = inherited
self.ndb = ndb self.ndb = ndb
self.generation = 0 self.generation = generation
self.name = name
} }
/// Only access temporarily! Do not store database references for longterm use. If it's a primitive type you /// Only access temporarily! Do not store database references for longterm use. If it's a primitive type you
@@ -68,32 +74,42 @@ class NdbTxn<T> {
deinit { deinit {
if self.generation != ndb.generation { if self.generation != ndb.generation {
//print("txn: OLD GENERATION (\(self.generation) != \(ndb.generation)), IGNORING") print("txn: OLD GENERATION (\(self.generation) != \(ndb.generation)), IGNORING")
return return
} }
if moved || inherited || ndb.closed { if inherited {
print("txn: not closing. inherited ")
return
}
if moved {
//print("txn: not closing. moved")
return
}
if ndb.is_closed {
print("txn: not closing. db closed")
return return
} }
#if TXNDEBUG #if TXNDEBUG
txn_count -= 1; txn_count -= 1;
print("txn: closing transaction \(txn_count)") print("txn: close gen\(generation) '\(name)' \(txn_count)")
#endif #endif
ndb_end_query(&self.txn) ndb_end_query(&self.txn)
//self.skip_close = true
Thread.current.threadDictionary.removeObject(forKey: "ndb_txn") Thread.current.threadDictionary.removeObject(forKey: "ndb_txn")
} }
// functor // functor
func map<Y>(_ transform: (T) -> Y) -> NdbTxn<Y> { func map<Y>(_ transform: (T) -> Y) -> NdbTxn<Y> {
self.moved = true self.moved = true
return .init(ndb: self.ndb, txn: self.txn, val: transform(val)) return .init(ndb: self.ndb, txn: self.txn, val: transform(val), generation: generation, inherited: inherited, name: self.name)
} }
// comonad!? // comonad!?
// useful for moving ownership of a transaction to another value // useful for moving ownership of a transaction to another value
func extend<Y>(_ with: (NdbTxn<T>) -> Y) -> NdbTxn<Y> { func extend<Y>(_ with: (NdbTxn<T>) -> Y) -> NdbTxn<Y> {
self.moved = true self.moved = true
return .init(ndb: self.ndb, txn: self.txn, val: with(self)) return .init(ndb: self.ndb, txn: self.txn, val: with(self), generation: generation, inherited: inherited, name: self.name)
} }
} }
@@ -116,7 +132,7 @@ extension NdbTxn where T: OptionalType {
return nil return nil
} }
self.moved = true self.moved = true
return NdbTxn<T.Wrapped>(ndb: self.ndb, txn: self.txn, val: unwrappedVal) return NdbTxn<T.Wrapped>(ndb: self.ndb, txn: self.txn, val: unwrappedVal, generation: generation, inherited: inherited, name: name)
} }
} }