From 7b1ecf79a6f67dcf2edf08dd75006235ea9d1715 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Thu, 11 Mar 2021 10:13:07 -0500 Subject: [PATCH] Fix race between err chan send and re-queue Signed-off-by: Monis Khan --- .../impersonatorconfig/impersonator_config.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index a06e44dc..389905d7 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -378,16 +378,18 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted(syncCtx contr } c.serverStopCh = make(chan struct{}) - c.errorCh = make(chan error) + // use a buffered channel so that startImpersonatorFunc can send + // on it without coordinating with the main controller go routine + c.errorCh = make(chan error, 1) // startImpersonatorFunc will block until the server shuts down (or fails to start), so run it in the background. go func() { - startOrStopErr := startImpersonatorFunc(c.serverStopCh) - // The server has stopped, so enqueue ourselves for another sync, so we can - // try to start the server again as quickly as possible. - syncCtx.Queue.AddRateLimited(syncCtx.Key) // TODO this a race because the main controller go routine could run and complete before we send on the err chan + // The server has stopped, so enqueue ourselves for another sync, + // so we can try to start the server again as quickly as possible. + defer syncCtx.Queue.AddRateLimited(syncCtx.Key) + // Forward any errors returned by startImpersonatorFunc on the errorCh. - c.errorCh <- startOrStopErr + c.errorCh <- startImpersonatorFunc(c.serverStopCh) }() return nil