Merge pull request #3690 from alltheseas/fix/post-compact-sigbus-crash
fix: delete stale lock.mdb during compaction to prevent SIGBUS crash
This commit is contained in:
@@ -129,4 +129,46 @@ final class NdbCompactionTests: XCTestCase {
|
|||||||
"Temp directory should be cleaned up after successful compaction"
|
"Temp directory should be cleaned up after successful compaction"
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MARK: - compact_if_needed: lock.mdb removal
|
||||||
|
|
||||||
|
func testCompactIfNeeded_removesLockFileAfterCompaction() {
|
||||||
|
// Given: a real Ndb database that has been opened and closed (so lock.mdb exists)
|
||||||
|
let dbPath = testDirectory.appendingPathComponent("lock_test_db").path
|
||||||
|
try? FileManager.default.createDirectory(atPath: dbPath, withIntermediateDirectories: true)
|
||||||
|
|
||||||
|
guard let ndb = Ndb(path: dbPath) else {
|
||||||
|
XCTFail("Could not open Ndb at \(dbPath)")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
ndb.close()
|
||||||
|
|
||||||
|
let lockPath = "\(dbPath)/lock.mdb"
|
||||||
|
XCTAssertTrue(
|
||||||
|
FileManager.default.fileExists(atPath: lockPath),
|
||||||
|
"lock.mdb should exist after opening and closing Ndb"
|
||||||
|
)
|
||||||
|
|
||||||
|
// When: compact_if_needed runs
|
||||||
|
Ndb.set_compact_on_next_launch()
|
||||||
|
Ndb.compact_if_needed(db_path: dbPath)
|
||||||
|
|
||||||
|
// Then: lock.mdb should NOT exist (it was deleted during compaction)
|
||||||
|
XCTAssertFalse(
|
||||||
|
FileManager.default.fileExists(atPath: lockPath),
|
||||||
|
"lock.mdb should be removed after compaction to prevent stale reader-table crashes"
|
||||||
|
)
|
||||||
|
|
||||||
|
// And: LMDB recreates a fresh lock.mdb when re-opened
|
||||||
|
guard let reopenedNdb = Ndb(path: dbPath) else {
|
||||||
|
XCTFail("Could not re-open Ndb after compaction — database may be corrupt")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
reopenedNdb.close()
|
||||||
|
|
||||||
|
XCTAssertTrue(
|
||||||
|
FileManager.default.fileExists(atPath: lockPath),
|
||||||
|
"lock.mdb should be recreated by LMDB after opening the compacted database"
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -109,6 +109,25 @@ extension Ndb {
|
|||||||
let originalDataMdb = URL(fileURLWithPath: "\(path)/\(main_db_file_name)")
|
let originalDataMdb = URL(fileURLWithPath: "\(path)/\(main_db_file_name)")
|
||||||
let compactedDataMdb = URL(fileURLWithPath: "\(tempPath)/\(main_db_file_name)")
|
let compactedDataMdb = URL(fileURLWithPath: "\(tempPath)/\(main_db_file_name)")
|
||||||
|
|
||||||
|
// Validate the compacted file before replacing the original.
|
||||||
|
let originalSize = (try? FileManager.default.attributesOfItem(atPath: originalDataMdb.path)[.size] as? Int) ?? 0
|
||||||
|
let compactedSize = (try? FileManager.default.attributesOfItem(atPath: compactedDataMdb.path)[.size] as? Int) ?? 0
|
||||||
|
Log.info("compact_if_needed: original=%d bytes, compacted=%d bytes", for: .storage, originalSize, compactedSize)
|
||||||
|
|
||||||
|
guard compactedSize > 0 else {
|
||||||
|
Log.error("compact_if_needed: compacted file is missing or empty — aborting", for: .storage)
|
||||||
|
try? FileManager.default.removeItem(atPath: tempPath)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Delete the stale lock.mdb BEFORE replacing data.mdb.
|
||||||
|
// The temp Ndb wrote reader-table / txn state into lock.mdb that references
|
||||||
|
// pages in the old data.mdb. After data.mdb is replaced with the smaller
|
||||||
|
// compacted copy, those page references become invalid and cause SIGBUS.
|
||||||
|
// LMDB will recreate a fresh lock file on the next open.
|
||||||
|
let lockPath = "\(path)/lock.mdb"
|
||||||
|
try? FileManager.default.removeItem(atPath: lockPath)
|
||||||
|
|
||||||
do {
|
do {
|
||||||
_ = try FileManager.default.replaceItemAt(
|
_ = try FileManager.default.replaceItemAt(
|
||||||
originalDataMdb,
|
originalDataMdb,
|
||||||
@@ -116,13 +135,22 @@ extension Ndb {
|
|||||||
backupItemName: nil,
|
backupItemName: nil,
|
||||||
options: [.usingNewMetadataOnly]
|
options: [.usingNewMetadataOnly]
|
||||||
)
|
)
|
||||||
Log.info("NostrDB compacted successfully", for: .storage)
|
|
||||||
} catch {
|
} catch {
|
||||||
Log.error("compact_if_needed: failed to replace db file: %@", for: .storage, String(describing: error))
|
Log.error("compact_if_needed: failed to replace db file: %@", for: .storage, String(describing: error))
|
||||||
try? FileManager.default.removeItem(atPath: tempPath)
|
try? FileManager.default.removeItem(atPath: tempPath)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Post-replace sanity check: verify the destination file exists with the expected size.
|
||||||
|
let finalSize = (try? FileManager.default.attributesOfItem(atPath: originalDataMdb.path)[.size] as? Int) ?? 0
|
||||||
|
if finalSize != compactedSize {
|
||||||
|
Log.error("compact_if_needed: post-replace size mismatch — expected %d, got %d", for: .storage, compactedSize, finalSize)
|
||||||
|
try? FileManager.default.removeItem(atPath: tempPath)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
Log.info("NostrDB compacted successfully", for: .storage)
|
||||||
|
|
||||||
// Clean up the temp directory (any remaining files such as lock.mdb).
|
// Clean up the temp directory (any remaining files such as lock.mdb).
|
||||||
try? FileManager.default.removeItem(atPath: tempPath)
|
try? FileManager.default.removeItem(atPath: tempPath)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user