Skip to content

Commit 663bc79

Browse files
donislawdevclaude
andcommitted
guard: say which road the stopped run hole came by
The hole this guard is about had two roads to it and the guard could not tell them apart. drain asks about cancellation AFTER taking an index, so the writer holding index zero can lose the processor in between, come back to a cancelled context and return without ever reaching its generator. Same hole, same manifest, weaker proof - a file that never started is not a file cut off half way. Measured with tools/probes/stoprace -held before changing anything: never entered 0 times in 50 idle runs and 0 in 25 starved ones. So the second road is one the construction allows and the machine does not take, and that number decided the shape of this change. The cancellation condition is therefore NOT rebuilt. Requiring the generator to have started before cancelling would be a change with no measured effect that carries its own risk, since a generator that never started would end the run in success. What is added instead is a net: the generator records that it was reached and the guard asserts it, as a fourth assertion beside the three it already makes about its own reach. This turns "measured once, did not happen" into "cannot happen unnoticed", which matters because the sizes and the cancellation condition above it are both things a later change can move. Measured after: 50 runs including 30 under CPU starvation, zero failures, the prefix mutation still reddens it, race detector clean. The assertion has no mutation of its own and that is deliberate - no mutation of product code reddens it alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1f6f004 commit 663bc79

2 files changed

Lines changed: 33 additions & 3 deletions

File tree

‎internal/guard/heldopen_test.go‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"io"
7+
"sync/atomic"
78
"time"
89

910
"github.com/donislawdev/TestingFilesGenerator/internal/format"
@@ -27,7 +28,24 @@ import (
2728
// This is put in place AFTER planning, on one PlannedFile, so it never enters
2829
// the registry and no other file sees it. Every step of the engine below
2930
// planning is the one that ships.
30-
type heldOpenGenerator struct{}
31+
type heldOpenGenerator struct{ entered atomic.Bool }
32+
33+
// Started says whether this generator was ever reached.
34+
//
35+
// It exists because there are TWO roads to the hole and the guard could not
36+
// tell them apart. drain asks ctx.Err() AFTER taking an index, so the writer
37+
// holding index zero can be descheduled in between, come back to a cancelled
38+
// context and return without ever writing - leaving the same hole for a
39+
// different reason. A file that never started is not a file cut off half way,
40+
// and the second road proves less than the first.
41+
//
42+
// Measured 2026-09-08 with tools/probes/stoprace before this was added: never
43+
// entered 0 times in 50 idle runs and 0 in 25 starved ones, so the second road
44+
// is one the construction allows and the machine does not take. This assertion
45+
// is therefore a net rather than a patch - it turns "measured once, did not
46+
// happen" into "cannot happen unnoticed", which matters because the sizes and
47+
// the cancellation condition above it are both things a later change can move.
48+
func (g *heldOpenGenerator) Started() bool { return g.entered.Load() }
3149

3250
// heldOpenDeadline is a safety net and not the mechanism.
3351
//
@@ -53,7 +71,8 @@ func (*heldOpenGenerator) Plan(format.Request) (format.Plan, error) {
5371
// so the engine sees the same thing it would see from any format that was cut
5472
// off - the temporary file is removed, no entry claims the file, and the index
5573
// stays empty.
56-
func (*heldOpenGenerator) Write(ctx context.Context, _ io.Writer, _ format.Plan) error {
74+
func (g *heldOpenGenerator) Write(ctx context.Context, _ io.Writer, _ format.Plan) error {
75+
g.entered.Store(true)
5776
select {
5877
case <-ctx.Done():
5978
return ctx.Err()

‎internal/guard/safety_test.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,8 @@ func TestARunStoppedPartWayNamesEveryFileThatFinished(t *testing.T) {
263263
// renamed into place while this one is still owed - which is the only
264264
// shape a sequential loop could not produce, and the shape this guard
265265
// exists to be about.
266-
planned[0].Desc.Generator = &heldOpenGenerator{}
266+
held := &heldOpenGenerator{}
267+
planned[0].Desc.Generator = held
267268

268269
res, runErr := engine.Run(ctx, planned, opt)
269270
if runErr == nil {
@@ -315,6 +316,16 @@ func TestARunStoppedPartWayNamesEveryFileThatFinished(t *testing.T) {
315316
"the survivors are a prefix and nothing here was cut off half way - which "+
316317
"is not the case this guard is about", planned[0].Name)
317318
}
319+
// The fourth of these, and the one that says WHICH ROAD the hole came by.
320+
// Without it a run where index zero was never begun reads exactly like a
321+
// run where it was cut off half way: same hole, same manifest, weaker
322+
// proof. See heldOpenGenerator.Started for the measurement.
323+
if !held.Started() {
324+
t.Fatalf("%s never reached its generator, so it was never begun rather than cut "+
325+
"off half way. The hole is there and this guard is not the one that proves "+
326+
"it - drain asks about cancellation after taking an index, and this writer "+
327+
"lost the processor in between", planned[0].Name)
328+
}
318329

319330
for name := range onDisk {
320331
if !claimed[name] {

0 commit comments

Comments
 (0)