From 34badea0d92ad87bae8a72939000a7186fcdb352 Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Sun, 12 Jul 2026 12:08:34 +1000 Subject: [PATCH] fix(dhcp): wait for renewal loop to quiesce before deleting lease on release (#509) handleRelease cancelled the per-lease renewal goroutine but deleted the KV entry without waiting for it to exit. An in-flight reaffirm/renew Put could land after the Delete, re-creating the lease and leaking a KV record. Add a per-loop done channel and block on it after cancel so any pending Put completes before the delete, making DrainAll's release path deterministically synchronous. --- spinifex/network/external/dhcp/manager.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/spinifex/network/external/dhcp/manager.go b/spinifex/network/external/dhcp/manager.go index 9e647cac1..ceb0c5886 100644 --- a/spinifex/network/external/dhcp/manager.go +++ b/spinifex/network/external/dhcp/manager.go @@ -74,9 +74,11 @@ type Manager struct { // leaseLoop is the handle stored in Manager.loops. Pointer-identity lets // run() distinguish its own loop from a successor (e.g. re-acquired lease -// with the same client-id). +// with the same client-id). done closes when run() exits so release can +// wait for an in-flight renewal Put to quiesce before deleting the KV entry. type leaseLoop struct { cancel context.CancelFunc + done chan struct{} } // NewManager constructs a Manager. Start must be called before @@ -220,7 +222,7 @@ func (m *Manager) spawnLoop(e Entry, reaffirm bool) { delete(m.loops, e.Lease.ClientID) } loopCtx, cancel := context.WithCancel(m.parentCtx) - loop := &leaseLoop{cancel: cancel} + loop := &leaseLoop{cancel: cancel, done: make(chan struct{})} m.loops[e.Lease.ClientID] = loop m.wg.Add(1) m.mu.Unlock() @@ -230,6 +232,7 @@ func (m *Manager) spawnLoop(e Entry, reaffirm bool) { func (m *Manager) run(ctx context.Context, self *leaseLoop, e Entry, reaffirm bool) { defer m.wg.Done() + defer close(self.done) defer func() { m.mu.Lock() if cur, ok := m.loops[e.Lease.ClientID]; ok && cur == self { @@ -604,12 +607,20 @@ func (m *Manager) handleRelease(ctx context.Context, clientID string) error { } m.mu.Lock() - if loop, ok := m.loops[clientID]; ok { + loop, ok := m.loops[clientID] + if ok { loop.cancel() delete(m.loops, clientID) } m.mu.Unlock() + // Wait for the renewal goroutine to exit so any in-flight reaffirm/renew + // Put has landed before we Delete; otherwise a late Put re-creates the + // lease after drain, leaking a KV record. + if ok { + <-loop.done + } + if err := m.client.Release(ctx, entry.Lease); err != nil { slog.Warn("dhcp manager: client release failed; deleting KV entry anyway", "client_id", clientID, "err", err) }