guard: make the stopped run hole certain instead of raced for

internal/guard/heldopen_test.go

@@ -0,0 +1,64 @@
+package guard
+
+import (
+	"context"
+	"errors"
+	"io"
+	"time"
+
+	"github.com/donislawdev/TestingFilesGenerator/internal/format"
+)
+
+// heldOpenGenerator is a file that never finishes.
+//
+// It exists for one guard - TestARunStoppedPartWayNamesEveryFileThatFinished -
+// which needs a run where one writer is still owed bytes while its neighbours
+// have already been renamed into place. That state used to be bought with
+// size, a first file sixteen times the size of the rest, and that was a race:
+// which writer finishes first is decided by which one gets a processor, not by
+// how much work it was given. Measured on 2026-09-08, a starved machine failed
+// it fourteen times in twenty five. The note above that guard carries the
+// numbers.
+//
+// Writing nothing and waiting removes the race rather than widening it. No
+// amount of scheduling pressure can make this file finish, so the hole is a
+// property of the plan instead of an outcome of a contest.
+//
+// This is put in place AFTER planning, on one PlannedFile, so it never enters
+// the registry and no other file sees it. Every step of the engine below
+// planning is the one that ships.
+type heldOpenGenerator struct{}
+
+// heldOpenDeadline is a safety net and not the mechanism.
+//
+// The cancellation this generator waits for arrives as soon as any other
+// writer finishes a file, which on an idle machine is eleven milliseconds and
+// on the most starved machine measured was under fifteen seconds. If it never
+// arrives at all the run is deadlocked, and a deadlock would hang the whole
+// package until the suite timeout kills it with no word about which test was
+// stuck. Failing here instead says what happened, on the guard that caused it.
+const heldOpenDeadline = 90 * time.Second
+
+// Plan is never called. This generator replaces the real one after planning,
+// so the file it belongs to already carries the plan the txt format made.
+// Refusing rather than returning a zero plan, because a zero plan would let a
+// future caller get a silently empty file out of this.
+func (*heldOpenGenerator) Plan(format.Request) (format.Plan, error) {
+	return format.Plan{}, errors.New("guard: this generator is installed after planning and has no plan of its own")
+}
+
+// Write emits nothing and returns when the run is cancelled.
+//
+// Returning the context error is what a generator interrupted half way does,
+// so the engine sees the same thing it would see from any format that was cut
+// off - the temporary file is removed, no entry claims the file, and the index
+// stays empty.
+func (*heldOpenGenerator) Write(ctx context.Context, _ io.Writer, _ format.Plan) error {
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-time.After(heldOpenDeadline):
+		return errors.New("guard: the run was never cancelled, so no other writer ever " +
+			"finished a file and the hole this guard is about could not occur")
+	}
+}

internal/guard/safety_test.go

@@ -187,17 +187,51 @@ func TestAnInterruptedRunLeavesNoPartialFileAndStillWritesAManifest(t *testing.T
 // cut off half way, what the run left behind was a prefix after all, and the
 // mutation that puts the prefix behaviour back could not redden anything.
 // A guard that names the defect and cannot meet it is the shape this project
-// has recorded twice. So the plan below is built to guarantee the hole: the
-// FIRST file is sixteen times the size of the rest, so a later one always
-// finishes first, the cancellation always lands while file one is still being
-// written, and index one is always missing from what survives.
+// has recorded twice.
+//
+// The second version bought the hole with SIZE: the first file was sixteen
+// times the size of the rest, so a smaller one was expected to always finish
+// first. That version was flaky, roughly one run in four inside a full suite,
+// and the fix is not a bigger first file. Measured on 2026-09-08 with
+// tools/probes/stoprace, fifty runs per condition:
+//
+//	idle machine                 0 failures in 50, and the big file had 165 ms
+//	                             of margin it never came close to using
+//	heavy disk writing beside it  0 failures in 25
+//	CPU starved, 24 busy loops    14 failures in 25
+//
+// So the flakiness was never about how fast the disk is. Size buys margin in
+// WORK, and what decides which writer finishes first is which writer gets a
+// processor. Starve the machine and the time to the first finished file goes
+// from 11 ms to 5.8 s, three orders of magnitude, at which point sixteen times
+// the work means nothing at all. A full suite runs packages beside each other
+// and builds binaries in sub processes, so a starved machine is the normal
+// condition rather than the exotic one.
+//
+// This version does not race. The file at index zero is given a generator that
+// writes nothing and returns only once the run is cancelled, so it can never
+// finish no matter who gets a processor. Descriptor is a value and every
+// planned file carries its own copy, so this replaces the generator for that
+// one file and leaves the other thirty one writing real bytes through the real
+// registry. Every step of the engine below Plan is the one that ships.
 func TestARunStoppedPartWayNamesEveryFileThatFinished(t *testing.T) {
 	dir := t.TempDir()
 
 	// Raised so several writers exist wherever this runs, because with one
 	// writer a stopped run leaves a prefix and the hole this guard is about
-	// cannot occur.
+	// cannot occur. It is also what keeps the blocked file below from being a
+	// deadlock: somebody other than the blocked writer has to finish a file,
+	// or the cancellation this guard waits for is never sent.
+	//
+	// Asserted rather than assumed. The pool is min(GOMAXPROCS, files), so a
+	// build that ever answered one here would hang instead of failing, and
+	// this guard has already been bitten once by a condition it took for
+	// granted.
 	defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(8))
+	if n := runtime.GOMAXPROCS(0); n < 2 {
+		t.Fatalf("this guard needs at least two writers and GOMAXPROCS is %d, so the "+
+			"file held open below would wait for a cancellation nobody can send", n)
+	}
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -214,19 +248,23 @@ func TestARunStoppedPartWayNamesEveryFileThatFinished(t *testing.T) {
 			}
 		},
 	}
-	// One big file and thirty one small ones, and the order is the whole
-	// point. Every writer starts at once, one of the small ones finishes long
-	// before the big one can, and the cancellation that follows cuts the big
-	// one off - leaving a finished file at a HIGHER index than one that never
-	// finished, which is the only shape a sequential loop could not produce.
-	sizes := append([]int64{32 << 20}, engine.Uniform(31, 2<<20)...)
+	// Thirty two files of one size. The asymmetry that used to live here was
+	// in the sizes and it is now in the generator, which is the whole of the
+	// fix - see the note above the function.
 	planned, err := engine.Plan([]engine.Target{{
-		ID: "files", Format: "txt", Sizes: sizes,
+		ID: "files", Format: "txt", Sizes: engine.Uniform(32, 512<<10),
 	}}, opt)
 	if err != nil {
 		t.Fatalf("planning: %v", err)
 	}
 
+	// Index zero is held open for the length of the run. Its writer reaches
+	// the generator, writes nothing and waits, so a file at a HIGHER index is
+	// renamed into place while this one is still owed - which is the only
+	// shape a sequential loop could not produce, and the shape this guard
+	// exists to be about.
+	planned[0].Desc.Generator = &heldOpenGenerator{}
+
 	res, runErr := engine.Run(ctx, planned, opt)
 	if runErr == nil {
 		t.Fatal("the run was cancelled from inside itself and reported success")
@@ -273,9 +311,9 @@ func TestARunStoppedPartWayNamesEveryFileThatFinished(t *testing.T) {
 			"was never stopped part way", len(planned))
 	}
 	if onDisk[planned[0].Name] {
-		t.Fatalf("%s is the biggest file in the run and it finished anyway, so nothing "+
-			"here was cut off half way and the survivors are a prefix - which is not the "+
-			"case this guard is about", planned[0].Name)
+		t.Fatalf("%s was held open for the whole run and it is on the disk anyway, so "+
+			"the survivors are a prefix and nothing here was cut off half way - which "+
+			"is not the case this guard is about", planned[0].Name)
 	}
 
 	for name := range onDisk {