fix: resolve deadlock in database migration engine

Fixed a deadlock in the Migrator where Pending() and Migrate() methods
called CurrentVersion() while holding the lock, but CurrentVersion()
also tried to acquire the same lock. Created internal currentVersionLocked()
method for lock-safe access.

Also fixed unused imports in migrations.go and fixed NOT NULL constraint
in TestMigrateFromV1 test.

All tests pass including:
- TestMigrateIdempotent: running migrations on already-migrated DB is no-op
- TestMigrateFromV1: migrating from v1 to current version
- TestMigrationRollback: failed migrations roll back cleanly
- TestPreMigrationBackup: backups created before schema changes
- TestPendingMigrations: pending migrations correctly identified
- TestCurrentVersion: version queries work for all DB states
- TestBackupPruning: old backups pruned correctly
- TestOpenDBFullSequence: full 7-phase startup sequence

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jedarden 2026-04-06 11:02:08 -04:00
parent 9e45169859
commit f9632c7df4
3 changed files with 8 additions and 5 deletions

View file

@ -83,7 +83,12 @@ func (m *Migrator) Register(migrations ...Migration) {
func (m *Migrator) CurrentVersion(ctx context.Context) (int, error) {
m.mu.Lock()
defer m.mu.Unlock()
return m.currentVersionLocked(ctx)
}
// currentVersionLocked returns the current schema version without acquiring the lock.
// The caller must hold m.mu.
func (m *Migrator) currentVersionLocked(ctx context.Context) (int, error) {
// Check if schema_migrations table exists
var tableName string
err := m.db.QueryRowContext(ctx,
@ -114,7 +119,7 @@ func (m *Migrator) Pending(ctx context.Context) ([]Migration, error) {
m.mu.Lock()
defer m.mu.Unlock()
current, err := m.CurrentVersion(ctx)
current, err := m.currentVersionLocked(ctx)
if err != nil {
return nil, err
}
@ -135,7 +140,7 @@ func (m *Migrator) Migrate(ctx context.Context) error {
m.mu.Lock()
defer m.mu.Unlock()
current, err := m.CurrentVersion(ctx)
current, err := m.currentVersionLocked(ctx)
if err != nil {
return fmt.Errorf("get current version: %w", err)
}

View file

@ -130,7 +130,7 @@ func TestMigrateFromV1(t *testing.T) {
updated_at INTEGER NOT NULL
);
INSERT INTO auth (id, install_secret) VALUES (1, X'0000000000000000000000000000000000000000000000000000000000000000');
INSERT INTO auth (id, install_secret, updated_at) VALUES (1, X'0000000000000000000000000000000000000000000000000000000000000000', strftime('%s', 'now') * 1000);
INSERT INTO schema_migrations (version, applied_at, description)
VALUES (1, strftime('%s', 'now') * 1000, 'initial schema');

View file

@ -3,8 +3,6 @@ package db
import (
"database/sql"
"fmt"
"time"
)
// AllMigrations returns the complete list of schema migrations in order.