From cd8afa47d9fd41ea3b388241f60c77f28048d074 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 12 Nov 2026 20:19:06 +0100 Subject: [PATCH] The teller reads the router's record outside its lock, so a catch-up never holds a warrant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit catchUp held the teller's lock across the router's record read (Where, through operatorask.Bucket and seatlist.HolderOf, which retries); with no answer it blocked the module's event pull, whose Said waits on that lock (met when an ask arrived inside a first view). OnAssignerAsk now catches up after it let go of the lock, catchUp takes the lock only once the record is read (an asking no longer open by then is left), and OnCurrent reads the records of the asks asked again outside the lock in the same way. Builds: design 47 G §717 (the teller raises the assigner's asks whether or not anything else answers, issue 1906); principle 3 Claude-Session: https://claude.ai/code/session_01NcbgfGKaxw984XyAPfLm4h --- .../cmd/mesh-delivery-teller/assignerasks.go | 8 +++-- .../mesh-delivery-teller/assignerasks_test.go | 35 +++++++++++++++++++ .../cmd/mesh-delivery-teller/teller.go | 33 +++++++++++++---- .../cmd/mesh-delivery-teller/viewanswers.go | 11 ++++-- 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks.go b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks.go index beb5039b..9b48d69e 100644 --- a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks.go +++ b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks.go @@ -133,8 +133,9 @@ func decodeAssignerAsk(key string, data []byte) (assignerAsked, error) { // raiseAssignerAsk asks the operator the assigner's ask at key, once per router id: an ask answered already (from the view or // the phone) is not asked; an earlier asking still open is taken back first; an asking already made is only caught up with -// at the router, and one that ended is not asked again, for the router drops a second ask under an id it keeps. -func (t *Teller) raiseAssignerAsk(key string) { +// at the router, and one that ended is not asked again, for the router drops a second ask under an id it keeps. An asking +// still open is answered as catchUp, for the caller to catch up with once it let go of the teller's lock. +func (t *Teller) raiseAssignerAsk(key string) (catchUp string) { a, found, err := t.assignerAsk(key) if err != nil { t.logf("%s: the ask is not raised: %v", key, err) @@ -170,7 +171,7 @@ func (t *Teller) raiseAssignerAsk(key string) { } switch { case open(k): - t.catchUp(id) + catchUp = id case endedUnanswered(k) && !byHand: // The key stands under an asking that ended at the channel without an answer, heard now or at the teller's start // (issue 1966: the nats yes on novox and three backup asks stood so after #195, and nothing asked them again): the @@ -200,6 +201,7 @@ func (t *Teller) raiseAssignerAsk(key string) { } t.assignerAsked(id) t.logf("%s: the assigner's ask %s is asked of the operator (%s)", key, id, a.Ask.Headline) + return } // OnAssignerAskGone handles a key the assigner took away from its asks (a hold it lifted by itself): the open router ask of diff --git a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks_test.go b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks_test.go index 66375ff4..fe7ca68d 100644 --- a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks_test.go +++ b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/assignerasks_test.go @@ -10,6 +10,9 @@ import ( "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-teller/tellerevents" "strings" "testing" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-assigner/asks" "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-contracts/go/clients/meshdeliveryclient" @@ -292,3 +295,35 @@ func anAssignerAskWrittenAgainAfterBeingTakenAwayIsAskedAgainUnderANewAsking(t * } } } + +// An ask the teller caught up with inside a first view reads the router's record outside the teller's lock (principle 3, nothing +// stalls silently): while that read waits on the bus, a warrant the module's event pull hands on (Said) is handled, not held +// behind it. The order met: the ask is asked, the first view hands the key on again, and its catch-up's read does not answer. +func TestACatchUpWaitingOnTheRouterHoldsNoWarrant(t *testing.T) { + b := newBed(t) + assigner, _ := withView(b) + key, value, _ := holdAsk(t) + assigner[key] = value + b.t.OnAssignerAsk(key) + record := b.client.Record + entered, release := make(chan struct{}), make(chan struct{}) + b.client.Record = func(id string) (*stdio.StateEntry, error) { + close(entered) + <-release + return record(id) + } + caught := make(chan struct{}) + go func() { defer close(caught); b.t.OnAssignerAsk(key) }() // the first view hands the key on: the ask is caught up with + <-entered + said := make(chan struct{}) + go func() { defer close(said); b.t.Said(occ.Done, oc.Warrant{Ask: "other"}, nil) }() + select { + case <-said: + case <-time.After(5 * time.Second): + close(release) + <-caught + t.Fatal("a warrant waited on the teller's lock while a catch-up read the router's record") + } + close(release) + <-caught +} diff --git a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/teller.go b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/teller.go index 460b4dd4..355534a1 100644 --- a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/teller.go +++ b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/teller.go @@ -179,13 +179,20 @@ func (t *Teller) cancelOpen(key, id string) { } // catchUp reads the router's record of an ask the teller made and hands its warrant on, as the router's event would: -// a warrant that came while the teller was away. +// a warrant that came while the teller was away. It is called without the teller's lock and reads the record before taking +// it: the read may wait on the bus (the seat's holder, the router's state), and a warrant the module's event pull hands on +// (Said) waits on that lock, so nothing stalls behind the read (principle 3). An asking no longer open by then is left. func (t *Teller) catchUp(id string) { state, w, err := t.Asker.Where(id) + t.mu.Lock() + defer t.mu.Unlock() if err != nil { t.logf("the router's record of the ask %s could not be read: %v", id, err) return } + if k, _, err := t.keptOf(id); err != nil || !open(k) { + return + } if w == nil { if state == "" { // The router holds no record of an ask the teller holds open: it refused it (a refused ask is not kept) while @@ -285,7 +292,7 @@ func (t *Teller) Said(h occ.Handled, w oc.Warrant, err error) { t.said(h, w, err) } -// said is Said under the lock (catchUp holds it). +// said is Said under the lock (catchUp takes it once the router's record is read). func (t *Teller) said(h occ.Handled, w oc.Warrant, err error) { if h == occ.Refused && w.Outcome == occ.OutcomeRefused { // The client answers Refused with the router's words for an ask the router refused at take-in (design 47 G §570, @@ -517,15 +524,16 @@ func (t *Teller) endWait(id string) { // more when the router holds no record of it: the router keeps none of a refused ask (review of 38116fe0). Up to // occ.MostOpen are asked, the most the router can have room for. func (t *Teller) OnCurrent() { + // The router's records are read outside the teller's lock (as catchUp's): a read that waits on the bus holds no warrant. t.mu.Lock() - defer t.mu.Unlock() - ws, rec, err := t.waiting() + ws, _, err := t.waiting() + t.mu.Unlock() if err != nil { t.logf("%v", err) return } - changed := false - for i, x := range ws { + unheard := map[string]bool{} + for _, x := range ws { if !x.Asked { continue } @@ -535,6 +543,19 @@ func (t *Teller) OnCurrent() { continue } if state == "" && w == nil { + unheard[x.ID] = true + } + } + t.mu.Lock() + defer t.mu.Unlock() + ws, rec, err := t.waiting() + if err != nil { + t.logf("%v", err) + return + } + changed := false + for i, x := range ws { + if x.Asked && unheard[x.ID] { t.logf("%s: the ask %s, asked again, is not on the router's record: its refusal was not heard, and it waits again", x.Key, x.ID) ws[i].Asked, changed = false, true } diff --git a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/viewanswers.go b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/viewanswers.go index 16cff681..5673e82e 100644 --- a/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/viewanswers.go +++ b/modules/mesh-delivery-teller/cmd/mesh-delivery-teller/viewanswers.go @@ -33,14 +33,21 @@ func (t *Teller) OnViewAnswer(key string) { // OnAssignerAsk handles a key put in the assigner's bucket asks: a view answer that came before its ask fills it now, and an // ask not answered yet is raised on the operator channel (issue 1906; assignerasks.go). func (t *Teller) OnAssignerAsk(key string) { + if id := t.onAssignerAsk(key); id != "" { + t.catchUp(id) // outside the lock: it reads the router's record + } +} + +// onAssignerAsk is OnAssignerAsk under the teller's lock, answering the asking to catch up with. +func (t *Teller) onAssignerAsk(key string) string { t.mu.Lock() defer t.mu.Unlock() if _, err := viewstate.ViewAnswerKey(key); err != nil { t.logf("the assigner's key %s is left and never answered from the view: %v", key, err) - return + return "" } t.fillFromView(key) - t.raiseAssignerAsk(key) + return t.raiseAssignerAsk(key) } // fillFromView writes the answer to the assigner's ask at askKey from the view's answer, when both are there and readable, the