structrually remove deadlock potential

.gitignore

@@ -38,3 +38,4 @@ dist/
 # specific tenants and their data shapes, which must not enter this
 # public repository (see CLAUDE.md content guidelines).
 docs/tasks/
+.raceout/

Makefile

@@ -51,8 +51,12 @@ protofmt: ## Format protobuf definitions.
 	buf format -w
 
 .PHONY: test
-test: ## Run the Go test suite used by CI.
-	go test -tags=baton_lambda_support -v ./...
+test: ## Run the ordinary Go test suite used by pull-request CI.
+	go test -tags=baton_lambda_support ./...
+
+.PHONY: test-full
+test-full: ## Run complete matrices and timing-sensitive soak iterations.
+	BATON_FULL_TESTS=1 BATON_CUT_SWEEP=full go test -tags=baton_lambda_support -count=1 -timeout=30m ./...
 
 # Two-artifact checkpoint compatibility matrix: builds the harness against
 # HEAD and a pinned past release, and exchanges mid-flight checkpoints in
@@ -236,7 +240,7 @@ bench: ## Run curated checkpoint and medium full-sync benchmarks.
 
 .PHONY: scheduler-soak
 scheduler-soak: ## Run randomized scheduler cases under race detection.
-	BATON_SOAK_ITERATIONS=$(SOAK_ITERATIONS) go test -race -v -count=1 -timeout=30m -run TestSchedulerSoakRandomizedFanoutWithFailures ./pkg/sync
+	BATON_FULL_TESTS=1 BATON_SOAK_ITERATIONS=$(SOAK_ITERATIONS) go test -race -v -count=1 -timeout=30m -run TestSchedulerSoakRandomizedFanoutWithFailures ./pkg/sync
 
 .PHONY: errorfs-soak
 errorfs-soak: ## Sweep whole-sync Pebble crash points using errorfs.

docs/TESTING.md

@@ -9,12 +9,26 @@ Run `make help` for the current target list.
 ## CI-equivalent tests
 
 `make test` runs the ordinary Go test suite with the same build tag used by
-CI. Pull-request CI also runs lint, protobuf checks, the full build, and
-`make race-shard-audit` (see [Race shards](#race-shards)).
+Linux pull-request CI. Windows CI additionally uses `-short` because its
+filesystem is substantially slower. Pull-request CI also runs lint, protobuf
+checks, bounded chaos checks, the full build, and `make race-shard-audit` (see
+[Race shards](#race-shards)).
 
 Tests in this tier should be deterministic, self-contained, and reasonably
-fast. A test that only skips on Windows with `testing.Short()` is still a CI
-test on the other platforms.
+fast. A test that only skips with `testing.Short()` remains part of Linux CI;
+Windows uses the reduced mode because filesystem-heavy matrices are
+substantially slower there.
+
+## Full Go suite
+
+`make test-full` expands the topological resume/interrupt fixture matrices,
+differential seed sweeps, checkpoint-cut sweep, and WAL timing soak. Ordinary
+CI retains every algorithm and interruption mode over representative acyclic
+and cyclic graphs, evenly spaced checkpoint/response/expiry cuts, and 20
+WAL-race attempts. The full tier adds specialized graph fixtures and wider
+sampling. The six-seed randomized scheduler soak also runs here and in
+`make scheduler-soak`; ordinary CI retains the deterministic scheduler tests
+and the complete deterministic chaos corpora.
 
 ## Bounded checks omitted from CI
 

pkg/dotc1z/c1file.go

@@ -58,6 +58,15 @@ type C1File struct {
 	closed             bool
 	closedMu           sync.Mutex
 
+	// dbClosed publishes handle closed-ness to concurrent callers.
+	// rawDb and db are written once at construction and never
+	// reassigned on a live C1File, so readers on other goroutines can
+	// load them without synchronization; closeRawDB flips this instead
+	// of nil-ing the fields. closedMu cannot serve this purpose because
+	// Close holds it across finalize, which is tens of minutes on a
+	// whale-scale file, and every reader would block for the duration.
+	dbClosed atomic.Bool
+
 	// bulkLoad defers secondary-index creation on a freshly-created
 	// destination. When set, the per-table non-unique secondary indexes
 	// are dropped right after table creation (instant on the empty table)
@@ -640,7 +649,7 @@ func (c *C1File) Close(ctx context.Context) (retErr error) {
 	// then dirtying via an attached-db mutation still releases the
 	// SQLite handle and any FDs/goroutines it owns.
 	if !c.dbUpdated.Load() || c.readOnly {
-		if c.rawDb != nil {
+		if c.rawDBOpen() {
 			if err := c.closeRawDB(ctx); err != nil {
 				return cleanupDbDir(c.dbFilePath, err)
 			}
@@ -722,10 +731,14 @@ func (c *C1File) finalize(ctx context.Context) error {
 	l := ctxzap.Extract(finalizeCtx)
 
 	// Only WAL-checkpoint and close the raw DB if a handle is open.
-	// Some callers (notably TestC1ZDecoder) manually close c.rawDb to
+	// Some callers (notably TestC1ZDecoder) release c.rawDb themselves to
 	// force a checkpoint before calling Close — that path skips both
-	// operations here and proceeds directly to saveC1z.
-	if c.rawDb != nil {
+	// operations here and proceeds directly to saveC1z. Checkpointing a
+	// released handle would fail with sql.ErrConnDone and send us down the
+	// cleanupDbDir branch, deleting the working database instead of saving
+	// it, so this has to recognize a released handle however it was
+	// released.
+	if c.rawDBOpen() {
 		// CRITICAL: Force a full WAL checkpoint before closing the database.
 		// This ensures all WAL data is written back to the main database file
 		// and the writes are synced to disk. Without this, on filesystems with
@@ -801,22 +814,33 @@ func (c *C1File) finalize(ctx context.Context) error {
 	return nil
 }
 
-// closeRawDB wraps c.rawDb.Close with a span and drops the handle
-// references on the C1File so callers do not have to repeat the
-// nil-out. Returns the error from rawDb.Close so error paths can
-// still propagate or log it.
+// rawDBOpen reports whether the SQLite handle is still usable.
+//
+// Two idioms release the handle and both must read as closed here.
+// closeRawDB publishes closed-ness through dbClosed and leaves the
+// pointer in place, because concurrent readers race a nil-ing write
+// (that race is what dbClosed exists to fix). Some tests instead close
+// c.rawDb directly and nil it. A bare nil check would miss the first
+// idiom and let a caller issue queries against a closed *sql.DB.
+func (c *C1File) rawDBOpen() bool {
+	return c.rawDb != nil && !c.dbClosed.Load()
+}
+
+// closeRawDB wraps c.rawDb.Close with a span and marks the handles
+// closed so subsequent callers short-circuit. Returns the error from
+// rawDb.Close so error paths can still propagate or log it.
+//
+// The CAS makes the underlying Close happen at most once; the flag is
+// published before the Close so a racing caller fails closed rather
+// than entering a database/sql call that is about to be torn down.
 func (c *C1File) closeRawDB(ctx context.Context) error {
 	_, span := tracer.Start(ctx, "C1File.closeRawDB")
 	var err error
 	defer func() { uotel.EndSpanWithError(span, err) }()
-	if c.rawDb == nil {
+	if c.rawDb == nil || !c.dbClosed.CompareAndSwap(false, true) {
 		return nil
 	}
-	// Copy the rawDb to a local variable to avoid race conditions.
-	rawDb := c.rawDb
-	c.rawDb = nil
-	err = rawDb.Close()
-	c.db = nil
+	err = c.rawDb.Close()
 	return err
 }
 
@@ -1465,9 +1489,9 @@ func (c *C1File) countBySyncAndResourceType(
 	return out, nil
 }
 
-// validateDb ensures that the database has been opened.
+// validateDb ensures that the database has been opened and not yet closed.
 func (c *C1File) validateDb(ctx context.Context) error {
-	if c.db == nil {
+	if c.db == nil || c.dbClosed.Load() {
 		return ErrDbNotOpen
 	}
 

pkg/dotc1z/c1file_close_cancel_test.go

@@ -94,7 +94,7 @@ func TestC1FileCloseSurvivesCanceledCtx(t *testing.T) {
 // that even a Close called with a cancelled context fully releases
 // the underlying sql.DB handle. The cheap path's only ctx-bearing op
 // (closeRawDB → c.rawDb.Close) ignores cancellation; this test pins
-// the structural promise that c.rawDb ends up nil regardless.
+// the structural promise that the handle ends up closed regardless.
 func TestC1FileCloseReadOnlyClosesRawDb(t *testing.T) {
 	openCtx := t.Context()
 	testFilePath := filepath.Join(c1zTests.workingDir, "close-readonly.c1z")
@@ -114,7 +114,7 @@ func TestC1FileCloseReadOnlyClosesRawDb(t *testing.T) {
 	cancelCtx, cancel := context.WithCancel(openCtx)
 	cancel()
 	require.NoError(t, f2.Close(cancelCtx))
-	require.Nil(t, f2.rawDb, "rawDb must be nil-ed out by the cheap-path close even on cancelled ctx")
+	require.True(t, f2.dbClosed.Load(), "rawDb must be closed by the cheap-path close even on cancelled ctx")
 	require.True(t, f2.closed, "c.closed must be set after a successful cheap-path close")
 }
 
@@ -142,6 +142,50 @@ func TestC1FileCloseReadOnlyButDirtyClosesRawDb(t *testing.T) {
 
 	err = f2.Close(openCtx)
 	require.ErrorIs(t, err, ErrReadOnly)
-	require.Nil(t, f2.rawDb, "rawDb must be closed before returning ErrReadOnly")
+	require.True(t, f2.dbClosed.Load(), "rawDb must be closed before returning ErrReadOnly")
 	require.True(t, f2.closed, "c.closed must be set after returning ErrReadOnly so a retry short-circuits")
 }
+
+// TestC1FileFinalizeSavesWhenHandleReleasedOutOfBand covers the
+// checkpoint-then-release idiom: a caller that WAL-checkpoints and
+// releases the sql.DB handle before calling Close must still get its
+// c1z written.
+//
+// finalize decides whether to checkpoint by asking whether the handle is
+// open. Once closed-ness moved to the dbClosed flag, a nil-pointer check
+// answered that question wrong: finalize would checkpoint a released
+// handle, get sql.ErrConnDone, and take the cleanupDbDir branch —
+// deleting the working database instead of saving it.
+func TestC1FileFinalizeSavesWhenHandleReleasedOutOfBand(t *testing.T) {
+	openCtx := t.Context()
+	testFilePath := filepath.Join(c1zTests.workingDir, "close-handle-released.c1z")
+
+	f, err := NewC1ZFile(openCtx, testFilePath)
+	require.NoError(t, err)
+	_, err = f.StartNewSync(openCtx, connectorstore.SyncTypeFull, "")
+	require.NoError(t, err)
+	require.NoError(t, f.PutResourceTypes(openCtx, v2.ResourceType_builder{Id: testResourceType}.Build()))
+	require.NoError(t, f.EndSync(openCtx))
+
+	// Checkpoint and release the handle out of band, exactly as a caller
+	// forcing a checkpoint before Close does.
+	_, _, _, err = f.truncateWAL(openCtx)
+	require.NoError(t, err)
+	require.NoError(t, f.closeRawDB(openCtx))
+	require.True(t, f.dbClosed.Load(), "fixture: the handle must read as closed")
+
+	require.NoError(t, f.Close(openCtx), "Close must skip the checkpoint and save the c1z")
+
+	info, err := os.Stat(testFilePath)
+	require.NoError(t, err)
+	require.Greater(t, info.Size(), int64(0))
+
+	f2, err := NewC1ZFile(openCtx, testFilePath)
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, f2.Close(context.Background())) })
+	resp, err := f2.GetResourceType(openCtx, reader_v2.ResourceTypesReaderServiceGetResourceTypeRequest_builder{
+		ResourceTypeId: testResourceType,
+	}.Build())
+	require.NoError(t, err)
+	require.Equal(t, testResourceType, resp.GetResourceType().GetId())
+}

pkg/dotc1z/clone_sync.go

@@ -233,13 +233,13 @@ func (c *C1File) SnapshotTo(ctx context.Context, outPath string, opts ...C1FOpti
 // connection. cloneCopy reads c.rawDb to take that one connection but never
 // mutates c.rawDb/c.db/c.currentSyncID/c.closed, so the live handle is left
 // exactly as it was found (plus the row-copy stall). Both entry points already
-// reject a closed handle; the nil-rawDb guard below is defensive against a
+// reject a closed handle; the validateDb guard below is defensive against a
 // future caller that does not.
 // errPrefix is the caller's error-message namespace ("clone-sync" /
 // "snapshot-to") so each entry point keeps its own observable error strings.
 func (c *C1File) cloneCopy(ctx context.Context, outPath string, syncID string, selectAll bool, errPrefix string, opts ...C1FOption) error {
-	if c.rawDb == nil {
-		return ErrDbNotOpen
+	if err := c.validateDb(ctx); err != nil {
+		return err
 	}
 
 	// Be sure that the output path is empty else return an error
@@ -277,11 +277,9 @@ func (c *C1File) cloneCopy(ctx context.Context, outPath string, syncID string, s
 	if err != nil {
 		return err
 	}
-	if err = initFile.rawDb.Close(); err != nil {
+	if err = initFile.closeRawDB(ctx); err != nil {
 		return err
 	}
-	initFile.rawDb = nil
-	initFile.db = nil
 
 	qCtx, canc := context.WithCancel(ctx)
 	defer canc()

pkg/dotc1z/convert_open.go

@@ -144,7 +144,7 @@ func (c *C1File) closeWithoutSave(ctx context.Context) error {
 	if c.closed {
 		return nil
 	}
-	if c.rawDb != nil {
+	if c.rawDBOpen() {
 		if err := c.closeRawDB(ctx); err != nil {
 			return err
 		}

pkg/dotc1z/copy_isolate_sync.go

@@ -97,8 +97,8 @@ func (c *C1File) CopyIsolateSync(ctx context.Context, outPath string, syncID str
 	ctx, span := tracer.Start(ctx, "C1File.CopyIsolateSync")
 	defer func() { uotel.EndSpanWithError(span, err) }()
 
-	if c.rawDb == nil {
-		return ErrDbNotOpen
+	if err = c.validateDb(ctx); err != nil {
+		return err
 	}
 	if c.dbFilePath == "" {
 		return fmt.Errorf("copy-isolate-sync: source working database path is not set")
@@ -223,7 +223,7 @@ func (c *C1File) CopyIsolateSync(ctx context.Context, outPath string, syncID str
 	// Close. Set once Close is invoked so we never double-close its rawDb.
 	finalized := false
 	defer func() {
-		if !finalized && copyFile.rawDb != nil {
+		if !finalized && copyFile.rawDBOpen() {
 			if closeErr := copyFile.closeRawDB(ctx); closeErr != nil {
 				err = errors.Join(err, closeErr)
 			}