From 468915ff9402ebecf0c9eabc5b6c17a1abb72943 Mon Sep 17 00:00:00 2001 From: Agustin Henze Date: Mon, 8 Sep 2025 15:47:56 +0200 Subject: [PATCH] Fix db recover when repo.RefList is nil It tries to find dangling references but repo has no reference list and it ended up with a nice trace ``` root@hostname:/usr/scratch/agustin/aptly-stress-tester# aptly db recover Recovering database... Checking database integrity... panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x13b9c24] goroutine 1 [running]: github.com/aptly-dev/aptly/cmd.Run.func1() /home/tin/projects/aptly/cmd/run.go:17 +0xc5 panic({0x15abf60?, 0x2da1650?}) /usr/local/go/src/runtime/panic.go:792 +0x132 github.com/aptly-dev/aptly/deb.(*PackageRefList).ForEach(...) /home/tin/projects/aptly/deb/reflist.go:83 github.com/aptly-dev/aptly/deb.FindDanglingReferences(...) /home/tin/projects/aptly/deb/find_dangling.go:15 github.com/aptly-dev/aptly/cmd.checkRepo(0xc0015827e0) /home/tin/projects/aptly/cmd/db_recover.go:63 +0xa4 github.com/aptly-dev/aptly/cmd.checkIntegrity.(*LocalRepoCollection).ForEach.func1({0xc0002fc2d0?, 0xc0010c2018?, 0x1?}, {0xc11004c000, 0xb9, 0xe0}) /home/tin/projects/aptly/deb/local.go:229 +0xa7 github.com/aptly-dev/aptly/database/goleveldb.(*storage).ProcessByPrefix(0xc0005ae140?, {0xc0010c2018, 0x1, 0x1}, 0xc00107be40) /home/tin/projects/aptly/database/goleveldb/storage.go:114 +0x184 github.com/aptly-dev/aptly/deb.(*LocalRepoCollection).ForEach(...) /home/tin/projects/aptly/deb/local.go:222 github.com/aptly-dev/aptly/cmd.checkIntegrity() /home/tin/projects/aptly/cmd/db_recover.go:51 +0x7f github.com/aptly-dev/aptly/cmd.aptlyDBRecover(0x40ed4e?, {0xc0003b5c30?, 0x2?, 0xc00057bdb0?}) /home/tin/projects/aptly/cmd/db_recover.go:27 +0x93 github.com/smira/commander.(*Command).Dispatch(0xc00052c900, {0xc0003b5c30, 0x0, 0x0}) /go/pkg/mod/github.com/smira/commander@v0.0.0-20140515201010-f408b00e68d5/commands.go:305 +0xd1 github.com/smira/commander.(*Command).Dispatch(0xc00052ca20, {0xc0003b5c30, 0x1, 0x1}) /go/pkg/mod/github.com/smira/commander@v0.0.0-20140515201010-f408b00e68d5/commands.go:283 +0x165 github.com/smira/commander.(*Command).Dispatch(0xc000516000, {0xc0003b5c20, 0x2, 0x2}) /go/pkg/mod/github.com/smira/commander@v0.0.0-20140515201010-f408b00e68d5/commands.go:283 +0x165 github.com/aptly-dev/aptly/cmd.Run(0xc000516000, {0xc000194280?, 0x0?, 0x24a5bc8?}, 0x1) /home/tin/projects/aptly/cmd/run.go:41 +0x1b9 main.main() /home/tin/projects/aptly/main.go:27 +0x10d ``` After applying the fix you get ``` root@hostname:~/aptly-stress-tester# aptly db recover Recovering database... Checking database integrity... Warning: Repo "stress_test_repo_3532" has no reference list (severely corrupted), initializing empty list Warning: Repo "stress_test_repo_9244" has no reference list (severely corrupted), initializing empty list ``` Also it tries better to avoid nil references in repos, and snapshots --- cmd/db_recover.go | 27 ++++++++++++++++++++++----- deb/local.go | 6 ++++++ deb/remote.go | 7 ++++++- deb/snapshot.go | 6 +++++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/cmd/db_recover.go b/cmd/db_recover.go index baf599c51..2aa621d21 100644 --- a/cmd/db_recover.go +++ b/cmd/db_recover.go @@ -57,20 +57,37 @@ func checkRepo(repo *deb.LocalRepo) error { err := repos.LoadComplete(repo) if err != nil { - return fmt.Errorf("load complete repo %q: %s", repo.Name, err) + // If we can't load the repo, it might be severely corrupted + // Log the error but continue with other repos + context.Progress().Printf("Warning: Cannot load repo %q: %s\n", repo.Name, err) + return nil } - dangling, err := deb.FindDanglingReferences(repo.RefList(), collectionFactory.PackageCollection()) + // Check if RefList is nil (severe corruption case) + refList := repo.RefList() + if refList == nil { + context.Progress().Printf("Warning: Repo %q has no reference list (severely corrupted), initializing empty list\n", repo.Name) + // Initialize with empty reflist + repo.UpdateRefList(deb.NewPackageRefList()) + if err = repos.Update(repo); err != nil { + return fmt.Errorf("update repo with empty reflist: %w", err) + } + return nil + } + + dangling, err := deb.FindDanglingReferences(refList, collectionFactory.PackageCollection()) if err != nil { - return fmt.Errorf("find dangling references: %w", err) + // If we can't find dangling references, log but continue + context.Progress().Printf("Warning: Cannot check dangling references for repo %q: %s\n", repo.Name, err) + return nil } - if len(dangling.Refs) > 0 { + if dangling != nil && len(dangling.Refs) > 0 { for _, ref := range dangling.Refs { context.Progress().Printf("Removing dangling database reference %q\n", ref) } - repo.UpdateRefList(repo.RefList().Subtract(dangling)) + repo.UpdateRefList(refList.Subtract(dangling)) if err = repos.Update(repo); err != nil { return fmt.Errorf("update repo: %w", err) diff --git a/deb/local.go b/deb/local.go index 4be460769..f4c6e39d8 100644 --- a/deb/local.go +++ b/deb/local.go @@ -160,9 +160,15 @@ func (collection *LocalRepoCollection) Add(repo *LocalRepo) error { func (collection *LocalRepoCollection) Update(repo *LocalRepo) error { batch := collection.db.CreateBatch() _ = batch.Put(repo.Key(), repo.Encode()) + if repo.packageRefs != nil { _ = batch.Put(repo.RefKey(), repo.packageRefs.Encode()) + } else { + // Delete RefKey if packageRefs is nil + // This prevents inconsistent state where RefKey exists but is corrupted + _ = batch.Delete(repo.RefKey()) } + return batch.Write() } diff --git a/deb/remote.go b/deb/remote.go index c6fe595c2..31ac1a2ec 100644 --- a/deb/remote.go +++ b/deb/remote.go @@ -847,11 +847,16 @@ func (collection *RemoteRepoCollection) Add(repo *RemoteRepo) error { // Update stores updated information about repo in DB func (collection *RemoteRepoCollection) Update(repo *RemoteRepo) error { batch := collection.db.CreateBatch() - _ = batch.Put(repo.Key(), repo.Encode()) + if repo.packageRefs != nil { _ = batch.Put(repo.RefKey(), repo.packageRefs.Encode()) + } else { + // Delete RefKey if packageRefs is nil + // This prevents inconsistent state where RefKey exists but is corrupted + _ = batch.Delete(repo.RefKey()) } + return batch.Write() } diff --git a/deb/snapshot.go b/deb/snapshot.go index 74f8865db..b96f3b400 100644 --- a/deb/snapshot.go +++ b/deb/snapshot.go @@ -227,10 +227,14 @@ func (collection *SnapshotCollection) Add(snapshot *Snapshot) error { // Update stores updated information about snapshot in DB func (collection *SnapshotCollection) Update(snapshot *Snapshot) error { batch := collection.db.CreateBatch() - _ = batch.Put(snapshot.Key(), snapshot.Encode()) + if snapshot.packageRefs != nil { _ = batch.Put(snapshot.RefKey(), snapshot.packageRefs.Encode()) + } else { + // Delete RefKey if packageRefs is nil + // This prevents inconsistent state where RefKey exists but is corrupted + _ = batch.Delete(snapshot.RefKey()) } return batch.Write()