Stop for loop by passing empty struct down channel Go

Question

I am attempting to create a poller in Go that spins up and every 24 hours executes a function.

I want to also be able to stop the polling, I'm attempting to do this by having a done channel and passing down an empty struct to stop the for loop.

In my tests, the for just loops infinitely and I can't seem to stop it, am I using the done channel incorrectly? The ticker case works as expected.

Poller struct {
    HandlerFunc HandlerFunc
    interval    *time.Ticker
    done        chan struct{}
}

func (p *Poller) Start() error {
    for {
        select {
        case <-p.interval.C:
            err := p.HandlerFunc()
            if err != nil {
                return err
            }
        case <-p.done:
            return nil
        }
    }
}

func (p *Poller) Stop() {
    p.done <- struct{}{}
}

Here is the test that's exeuting the code and causing the infinite loop.

poller := poller.NewPoller(
    testHandlerFunc,
    time.NewTicker(1*time.Millisecond),
)

err := poller.Start()
assert.Error(t, err)
poller.Stop()

The source shown looks fine, the issue is likely elsewhere in your code. — Adrian, Apr 03 '20 at 14:59
You almost always want to `close` a channel for cancellation. Your for loop here will return if it receives `<-p.done`, so it must be blocked somewhere else. — JimB, Apr 03 '20 at 14:59
where is the code that sets the `done` channel on the `Poller`? — JamesHalsall, Apr 03 '20 at 15:06
@pocockn `err := poller.Start()` is blocking. So `poller.Stop()` is never called. — Grigoriy Mikhalkin, Apr 03 '20 at 15:06

Grigoriy Mikhalkin · Accepted Answer · 2020-04-04T12:56:10.173

2

Seems like problem is in your use case, you calling poller.Start() in blocking maner, so poller.Stop() is never called. It's common, in go projects to call goroutine inside of Start/Run methods, so, in poller.Start(), i would do something like that:

func (p *Poller) Start() <-chan error {
    errc := make(chan error, 1 )

    go func() {
        defer close(errc)

        for {
            select {
            case <-p.interval.C:
                err := p.HandlerFunc()
                if err != nil {
                    errc <- err
                    return
                }
            case <-p.done:
                return
            }
        }
    }

    return errc
}

Also, there's no need to send empty struct to done channel. Closing channel like close(p.done) is more idiomatic for go.

edited Apr 04 '20 at 12:56

answered Apr 03 '20 at 15:18

Grigoriy Mikhalkin

5,035
1
18
36

1

I can't return the error from inside the go func() {}. Is it idiomatic to use go func() error {} ? Or is there a better way to deal with the error from the p.HandlerFunc()? – pocockn Apr 04 '20 at 12:39
It's better to use separate channel for errors. See updated example – Grigoriy Mikhalkin Apr 04 '20 at 12:56

score 0 · Answer 2 · answered Apr 03 '20 at 15:40

There is no explicit way in Go to broadcast an event to go routines for something like cancellation. Instead its idiomatic to create a channel that when closed signifies a message such as cancelling any work it has to do. Something like this is a viable pattern:

var done = make(chan struct{})

func cancelled() bool {
    select {
    case <-done:
        return true
    default:
        return false
    }
}

Go-routines can call cancelled to poll for a cancellation.

Then your main loop can respond to such an event but make sure you drain any channels that might cause go-routines to block.

for {
    select {
    case <-done:
    // Drain whatever channels you need to.
        for range someChannel { }
        return
    //.. Other cases
   }
}

Stop for loop by passing empty struct down channel Go

2 Answers2