From 1875ee7b8bfaaf8a9d050934cb467ea6ea53a846 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sat, 22 Aug 2026 10:26:21 +0100 Subject: [PATCH] fix(registry): defer context cancels on the refresh panic path A panic in listModels skipped the inline cancel, holding the timeout timer until it fired. Review follow-up from the v0.2.3 final verdict. --- registry.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/registry.go b/registry.go index d2f8486..14e4328 100644 --- a/registry.go +++ b/registry.go @@ -547,17 +547,22 @@ func (m *modelRegistry) refreshProvider(name string, st *providerState, adapter } }() + // Both cancels are deferred: this function has a recover above, so a + // panicking listModels would skip an inline cancel() and hold the + // timer until backgroundRefreshTimeout fires. Deferring costs nothing + // (the function ends right after the capture) and survives the panic + // path. fctx, cancel := context.WithTimeout(context.Background(), backgroundRefreshTimeout) + defer cancel() ids, err = adapter.listModels(fctx) - cancel() // Own timeout budget, review fix (SHOULD-4) — see warmFill's // identical comment: reusing fctx here would hand the metadata fetch // an already-spent context on every refresh whose listModels call // ran long, not just a slow or timed-out one. mctx, mcancel := context.WithTimeout(context.Background(), backgroundRefreshTimeout) + defer mcancel() m.captureModelMetadata(mctx, name, adapter, st) - mcancel() } // splitConfiguredProvider reports whether id has "prefix/rest" form where