Refactor: Protect Shutdown DB close with sync.Once to prevent double-close on repeated calls

[aaronbrethorst]

Context

PR #737 moved GtfsDB.Close() outside of shutdownOnce.Do() in Manager.Shutdown() to support the new context-based timeout pattern. This is correct for the primary use case (single shutdown call), but means a second Shutdown() call will:

1. Spawn an unnecessary goroutine for wg.Wait() (returns immediately since counter is 0)
2. Attempt to close the already-closed DB

Go's sql.DB.Close() is safe to call multiple times (returns nil), so this is not a bug. However, it would be cleaner to guard the DB close with its own sync.Once or track closure state with an atomic.Bool, matching the original idempotency guarantee.

Additionally, the DB close logic is duplicated between the two select branches (success and timeout). Extracting it into a deferred call or a helper would reduce duplication.

Suggested approach

func (manager *Manager) Shutdown(ctx context.Context) error {
    manager.shutdownOnce.Do(func() {
        close(manager.shutdownChan)
    })

    // Always close DB exactly once, regardless of path
    defer manager.closeDBOnce.Do(func() {
        if manager.GtfsDB != nil {
            if err := manager.GtfsDB.Close(); err != nil {
                logger := slog.Default().With(slog.String("component", "gtfs_manager"))
                logging.LogError(logger, "failed to close GTFS database", err)
            }
        }
    })

    done := make(chan struct{})
    go func() {
        manager.wg.Wait()
        close(done)
    }()

    select {
    case <-done:
        return nil
    case <-ctx.Done():
        return fmt.Errorf("shutdown timeout exceeded: %w", ctx.Err())
    }
}

Priority

Low — this is a cleanup, not a correctness issue.

[Jason10293]

Working on this!