goroutine not seeing context cancel?-CodePudding

I have two goroutines running at the same time.

At some point, I want my program to exit gracefully so I use the cancel() func to notify my goroutines that they need to be stopped, but only one of the two receive the message.

here is my main (simplified):

ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)

done := make(chan os.Signal, 1)
signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)

wg := &sync.WaitGroup{}
wg.Add(2)

go func() {
    err := eng.Watcher(ctx, wg)
    if err != nil {
        cancel()
    }
}()

go func() {
    err := eng.Suspender(ctx, wg)
    if err != nil {
        cancel()
    }
}()

<-done // wait for SIGINT / SIGTERM
log.Print("receive shutdown")
cancel()
wg.Wait()

log.Print("controller exited properly")

The Suspender goroutine exist successfully (here is the code):

package main

import (
    "context"
    "sync"
    "time"

    log "github.com/sirupsen/logrus"
    metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
    "k8s.io/client-go/util/retry"
)

func (eng *Engine) Suspender(ctx context.Context, wg *sync.WaitGroup) error {

    contextLogger := eng.logger.WithFields(log.Fields{
        "go-routine": "Suspender",
    })
    contextLogger.Info("starting Suspender goroutine")
    now := time.Now().In(eng.loc)

    for {
        select {
        case n := <-eng.Wl:
            //dostuff


        case <-ctx.Done():
            // The context is over, stop processing results
            contextLogger.Infof("goroutine Suspender canceled by context")
            return nil
        }
    }

}

and here is the func that is not receiving the context cancellation:

package main

import (
    "context"
    "sync"
    "time"

    log "github.com/sirupsen/logrus"
)

func (eng *Engine) Watcher(ctx context.Context, wg *sync.WaitGroup) error {
    contextLogger := eng.logger.WithFields(log.Fields{
        "go-routine":      "Watcher",
        "uptime-schedule": eng.upTimeSchedule,
    })
    contextLogger.Info("starting Watcher goroutine")

    ticker := time.NewTicker(time.Second * 30)
    for {
        select {
        case <-ctx.Done():
            contextLogger.Infof("goroutine watcher canceled by context")
            log.Printf("toto")
            return nil
        case <-ticker.C:
            
                //dostuff
            }
        }
    }
}

Can you please help me ?

Thanks :)

CodePudding user response：

Did you try it with an errgroup? It has context cancellation baked in:

ctx := context.Background()
ctx, cancel := context.WithCancel(ctx)
defer cancel()

done := make(chan os.Signal, 1)
signal.Notify(done, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)

// "golang.org/x/sync/errgroup"
wg, ctx := errgroup.WithContext(ctx)

wg.Go(func() error {
    return eng.Watcher(ctx, wg)
})

wg.Go(func() error {
    return eng.Suspender(ctx, wg)
})

wg.Go(func() error {
    defer cancel()
    <-done
    return nil
})

err := wg.Wait()
if err != nil {
    log.Print(err)
}

log.Print("receive shutdown")
log.Print("controller exited properly")

CodePudding user response：

On the surface the code looks good. The only thing I can think is that it's busy in "dostuff". It can be tricky to step through timing related code in the debugger so try adding some logging:

  case <-ticker.C:
     log.Println("doing stuff")
     //dostuff
     log.Println("done stuff")

(I also assume you are calling wg.Done() in your go-routines somewhere though if they are missing that would not be the cause of the problem you describe.)

CodePudding user response：

The code in Suspender and in Watcher doesn't decrement the waitgroup counter through the Done() method call - the reason behind the infinite execution.

And to be honest it's quite normal to forget such small things. That's why as a standard general practice in Go, it is suggested to use defer and handle things that are critical (and should be handled inside the function/method ) at the very beginning.

The updated implementation might look like

func (eng *Engine) Suspender(ctx context.Context, wg *sync.WaitGroup) error {
    defer wg.Done()

// ------------------------------------

func (eng *Engine) Watcher(ctx context.Context, wg *sync.WaitGroup) error {
    defer wg.Done()
    contextLogger := eng.logger.WithFields(log.Fields{

Also, another suggestion, looking at the main routine, it is always suggested to pass context by value to any go-routine or method calls (lambda) that are being invoked. This approach saves developers from a lot of program-related bugs that can't be noticed very easily.

go func(ctx context.Context) {
    err := eng.Watcher(ctx, wg)
    if err != nil {
        cancel()
    }
}(ctx)

Edit-1: (the exact solution)

Try passing the context using the value in the go routines as I mentioned earlier. Otherwise, both of the go routine will use a single context (because you are referencing it) and only one ctx.Done() will be fired. By passing ctx as a value 2 separate child contexts are created in Go. And while closing parent with cancel() - both children independently fires ctx.Done().