From b23bca73bc4368934dd846a70d2b8c0243957a37 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 7 Jul 2025 04:54:10 +0000 Subject: [PATCH] Fix sleepWithCancel and ensure closed channel * time.NewTicker will panic if the duration is 0. Make it return early if duration is 0. * Return a pre-closed channel in Wait() instead of nil. Ensures receiver will not block forever. Signed-off-by: Gabriel Adrian Samfira --- runner/pool/pool.go | 3 +++ workers/cache/tool_cache.go | 3 +++ workers/scaleset/scaleset.go | 8 ++++---- workers/scaleset/scaleset_listener.go | 10 +++++++--- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/runner/pool/pool.go b/runner/pool/pool.go index 17285e1d..9f6d3c0e 100644 --- a/runner/pool/pool.go +++ b/runner/pool/pool.go @@ -1369,6 +1369,9 @@ func (r *basePoolManager) deleteInstanceFromProvider(ctx context.Context, instan } func (r *basePoolManager) sleepWithCancel(sleepTime time.Duration) (canceled bool) { + if sleepTime == 0 { + return false + } ticker := time.NewTicker(sleepTime) defer ticker.Stop() diff --git a/workers/cache/tool_cache.go b/workers/cache/tool_cache.go index 727c82b4..6cbcc716 100644 --- a/workers/cache/tool_cache.go +++ b/workers/cache/tool_cache.go @@ -135,6 +135,9 @@ func (t *toolsUpdater) Reset() { } func (t *toolsUpdater) sleepWithCancel(sleepTime time.Duration) (canceled bool) { + if sleepTime == 0 { + return false + } ticker := time.NewTicker(sleepTime) defer ticker.Stop() diff --git a/workers/scaleset/scaleset.go b/workers/scaleset/scaleset.go index 5226d981..f5b34400 100644 --- a/workers/scaleset/scaleset.go +++ b/workers/scaleset/scaleset.go @@ -634,6 +634,9 @@ func (w *Worker) loop() { } func (w *Worker) sleepWithCancel(sleepTime time.Duration) (canceled bool) { + if sleepTime == 0 { + return false + } ticker := time.NewTicker(sleepTime) defer ticker.Stop() @@ -663,10 +666,7 @@ Loop: } continue } - // noop if already started. If the scaleset was just enabled, we need to - // start the listener here, or the <-w.listener.Wait() channel receive bellow - // will block forever, even if we start the listener, as a nil channel will - // block forever. + // noop if already started. if err := w.listener.Start(); err != nil { slog.ErrorContext(w.ctx, "error starting listener", "error", err, "consumer_id", w.consumerID) if canceled := w.sleepWithCancel(2 * time.Second); canceled { diff --git a/workers/scaleset/scaleset_listener.go b/workers/scaleset/scaleset_listener.go index d69092f5..1274ee59 100644 --- a/workers/scaleset/scaleset_listener.go +++ b/workers/scaleset/scaleset_listener.go @@ -25,6 +25,10 @@ import ( "github.com/cloudbase/garm/util/github/scalesets" ) +var closed = make(chan struct{}) + +func init() { close(closed) } + func newListener(ctx context.Context, scaleSetHelper scaleSetHelper) *scaleSetListener { return &scaleSetListener{ ctx: ctx, @@ -278,11 +282,11 @@ func (l *scaleSetListener) loop() { func (l *scaleSetListener) Wait() <-chan struct{} { l.mux.Lock() + defer l.mux.Unlock() + if !l.running { slog.DebugContext(l.ctx, "scale set listener is not running") - l.mux.Unlock() - return nil + return closed } - l.mux.Unlock() return l.loopExited }