From 3e302f72ce49788073cac1b32810095218020776 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 2 Oct 2026 14:06:25 +0000 Subject: [PATCH] Pin the close before the reopen in the archive sweep (closes #103) Add a test that keeps the archive's connection from before a sweep and checks that the sweep closed it. Without the close before the reopen, the reopen replaced the handle without closing it and one connection leaked per archive per sweep, yet every test passed. The sweeper's listing query now takes the sweep's context, and a comment on its cancel function says why it needs no lock. The handlers' type-filtered count of a webhook's remaining targets no longer exists: since each database target has its own archive file, deleting a target evicts that target's writer alone. Model: opus-5-5 --- internal/delivery/archive_sweeper.go | 9 ++++++-- internal/delivery/archive_sweeper_test.go | 28 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/delivery/archive_sweeper.go b/internal/delivery/archive_sweeper.go index 646b1e9..deb911d 100644 --- a/internal/delivery/archive_sweeper.go +++ b/internal/delivery/archive_sweeper.go @@ -45,8 +45,12 @@ type ArchiveSweeper struct { eng *Engine log *slog.Logger interval time.Duration - cancel context.CancelFunc - wg sync.WaitGroup + + // cancel needs no lock: fx runs start and then stop on the one + // goroutine that runs the app, so they never overlap. + cancel context.CancelFunc + + wg sync.WaitGroup } // NewArchiveSweeper creates the archive sweeper and registers @@ -163,6 +167,7 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) { var targets []database.Target err := s.db.DB(). + WithContext(ctx). Model(&database.Target{}). Where("type = ?", database.TargetTypeDatabase). Find(&targets).Error diff --git a/internal/delivery/archive_sweeper_test.go b/internal/delivery/archive_sweeper_test.go index 590c598..67c54f1 100644 --- a/internal/delivery/archive_sweeper_test.go +++ b/internal/delivery/archive_sweeper_test.go @@ -636,6 +636,34 @@ func TestArchiveSweep_LeavesArchiveClosed(t *testing.T) { ) } +// TestArchiveSweep_ClosesHandleBeforeReopening proves the sweep +// closes the handle it finds open before it reopens the file. +// TestArchiveSweep_LeavesArchiveClosed cannot see this: without the +// close, the reopen replaces the handle without closing it, the +// sweep then closes only the new one, and one connection leaks per +// archive per sweep. +func TestArchiveSweep_ClosesHandleBeforeReopening(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "archive.db") + + w := delivery.NewExportArchiveWriter( + path, archiveTestLogger(), 0, + ) + + require.NoError(t, w.Open(time.Hour)) + + before, err := w.DB().DB() + require.NoError(t, err) + + require.NoError(t, w.SweepExpired(time.Hour)) + + assert.Error( + t, before.PingContext(t.Context()), + "the handle open before the sweep must be closed by it", + ) +} + // TestArchiveSweep_ClosesHandleOfRegisteredWriter states the same // guarantee end to end, through the real sweeper and a writer the // registry keeps.