diff --git a/bundle/direct/dstate/state.go b/bundle/direct/dstate/state.go index d7c1e8e44e..60b698e49a 100644 --- a/bundle/direct/dstate/state.go +++ b/bundle/direct/dstate/state.go @@ -238,6 +238,29 @@ func (db *DeploymentState) Open(ctx context.Context, path string, withRecovery W panic(fmt.Sprintf("state already opened: %v, cannot open %v", db.Path, path)) } + err := db.unlockedOpen(ctx, path, withRecovery, withWrite) + if err != nil { + // A failed open must leave the receiver closed. unlockedOpen assigns + // db.Path before every fallible step, so without this the receiver stays + // half-initialized and the next Open on it hits the panic above instead + // of reporting the real error. + db.reset() + } + return err +} + +// reset returns the receiver to the not-opened state. Callers must hold db.mu. +func (db *DeploymentState) reset() { + if db.walFile != nil { + db.walFile.Close() + db.walFile = nil + } + db.Path = "" + db.Data = Database{} + db.stateIDs = nil +} + +func (db *DeploymentState) unlockedOpen(ctx context.Context, path string, withRecovery WithRecovery, withWrite WithWrite) error { db.Path = path data, err := os.ReadFile(db.Path) if err != nil { @@ -463,9 +486,7 @@ func (db *DeploymentState) Finalize(ctx context.Context) (resourcestate.Exported state := ExportStateFromData(db.Data) - db.Path = "" - db.Data = Database{} - db.stateIDs = nil + db.reset() return state, err } diff --git a/bundle/direct/dstate/state_test.go b/bundle/direct/dstate/state_test.go index 3b5dc06221..34ed9ff1c5 100644 --- a/bundle/direct/dstate/state_test.go +++ b/bundle/direct/dstate/state_test.go @@ -261,3 +261,23 @@ func TestGetOrInitLineageReadableBeforeWriteAndPersisted(t *testing.T) { assert.Equal(t, lineage, reopened.Data.Lineage) mustFinalize(t, &reopened) } + +// TestOpenFailureLeavesStateClosed pins that a failed Open leaves the receiver +// closed. Open assigns db.Path before the steps that can fail, so an unreadable +// state file used to leave Path set: the next Open on the same value panicked +// with "state already opened" instead of reporting the real error. +func TestOpenFailureLeavesStateClosed(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + require.NoError(t, os.WriteFile(path, []byte("{not json"), 0o600)) + + var db DeploymentState + require.Error(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true))) + assert.Empty(t, db.Path) + + // Once the state file is readable, the same receiver opens without panicking. + seed := `{"state_version":2,"cli_version":"0.1.2","lineage":"test-lineage","serial":1,"state":{}}` + require.NoError(t, os.WriteFile(path, []byte(seed), 0o600)) + require.NoError(t, db.Open(t.Context(), path, WithRecovery(true), WithWrite(true))) + assert.Equal(t, "test-lineage", db.Data.Lineage) + mustFinalize(t, &db) +}