diff --git a/internal/filling/filling.go b/internal/filling/filling.go index d8170fb..24ec940 100644 --- a/internal/filling/filling.go +++ b/internal/filling/filling.go @@ -170,12 +170,26 @@ func (e *Engine) Write(ctx context.Context, uid uint32, req Request) Answer { // write this engine refused or failed (issue 1873; ace, 2026-10-25 21:17: mesh-build-go's files were refused as not // named as filled 32 s before its want arrived, and nothing filled them after the apply that named them): a path // refused as not named that the declaration now names, a write refused while no provisioner account was named once -// one is, and any path whose write failed. Called after every apply (the node agent's afterApply): an event of the -// engine's own, never a timer. The client is told by touching the write socket, whose attributes it watches -// (mesh-provisioner-client socketwatch.go, IN_ATTRIB), and then fills every module it holds again. A refusal the -// declaration still does not end is kept for the next apply. It answers what it told the client of, for the journal; -// nothing when it told nothing. +// one is, and any path whose write failed. Called once a module's apply or removal settled (the node agent's +// Settled, after what runs holds it): an event of the engine's own, never a timer. It holds the apply's lock, as a +// write does, so a write judged by the declaration before the apply settled has recorded its refusal before Knock +// reads what is owed, and one judged after is judged by the settled declaration. The client is told by touching the +// write socket, whose attributes it watches (mesh-provisioner-client socketwatch.go, IN_ATTRIB), and then fills every +// module it holds again. A refusal the declaration still does not end is kept for the next apply. It answers what it +// told the client of, for the journal; nothing when it told nothing. With nothing owed it reads no declaration. func (e *Engine) Knock(socket string) (string, error) { + unlock, err := e.Lock() + if err != nil { + return "", fmt.Errorf("the apply's lock could not be taken, so the provisioner's client is not told to fill "+ + "again: %w", err) + } + defer unlock() + e.owed.mu.Lock() + nothing := len(e.owed.unnamed) == 0 && len(e.owed.failed) == 0 && !e.owed.noAccount + e.owed.mu.Unlock() + if nothing { + return "", nil + } d, err := e.Declared() if err != nil { return "", fmt.Errorf("this node's declaration cannot be read, so the provisioner's client is not told to "+ diff --git a/internal/filling/filling_test.go b/internal/filling/filling_test.go index 8c77d99..656d363 100644 --- a/internal/filling/filling_test.go +++ b/internal/filling/filling_test.go @@ -108,6 +108,7 @@ func TestAnUndeclaredPathIsRefused(t *testing.T) { `"filled_files":["/etc/m/m.conf","/etc/m/other.conf"]`, `"content":"y"}`, `"content":"y","owner-at-runtime":"owned"}`).Replace(withAccount) failing := errors.New("is a directory") + var mu2 sync.Mutex // guards the race row's count of declaration reads for _, row := range []struct { name string before string // the declaration the write is judged by @@ -158,6 +159,67 @@ func TestAnUndeclaredPathIsRefused(t *testing.T) { } }) } + + // The race (review of this fix): a write judged by the declaration before the apply settled, its refusal not yet + // recorded when Knock runs. Knock waits for the apply's lock the write holds, so it sees that refusal and knocks. + t.Run("a write still judging when the apply settles", func(t *testing.T) { + if err := os.Chtimes(socket, long, long); err != nil { + t.Fatal(err) + } + before, after := declared(t, withAccount), declared(t, namesOther) + judging, release := make(chan struct{}), make(chan struct{}) + var mu sync.Mutex + knockLocking := make(chan struct{}, 1) + calls := 0 + e := newEngine(t, withAccount, &mu) + e.Declared = func() (*declaration.Declaration, error) { + mu2.Lock() + calls++ + first := calls == 1 + mu2.Unlock() + if first { + close(judging) + <-release + return before, nil + } + return after, nil + } + e.Lock = func() (func(), error) { + mu2.Lock() + second := calls > 0 + mu2.Unlock() + if second { + knockLocking <- struct{}{} + } + mu.Lock() + return mu.Unlock, nil + } + wrote := make(chan Answer) + go func() { wrote <- e.Write(context.Background(), provisionerUID, Request{Path: "/etc/m/other.conf"}) }() + <-judging + type result struct { + told string + err error + } + knocked2 := make(chan result, 1) + go func() { told, err := e.Knock(socket); knocked2 <- result{told, err} }() + // Released once Knock waits for the lock, or once it has answered without taking it. + var r result + select { + case <-knockLocking: + close(release) + r = <-knocked2 + case r = <-knocked2: + close(release) + } + if a := <-wrote; a.Refused == "" { + t.Fatalf("the write was not refused: %+v", a) + } + if r.err != nil || r.told != "/etc/m/other.conf (now named as filled)" || !knocked() { + t.Fatalf("knock told %q (%v), socket touched %v: the refusal still being judged was missed", r.told, r.err, + knocked()) + } + }) } func TestAnotherAccountIsRefused(t *testing.T) {