91

I have two goroutines independently producing data, each sending it to a channel. In my main goroutine, I'd like to consume each of these outputs as they come in, but don't care the order in which they come in. Each channel will close itself when it has exhausted its output. While the select statement is the nicest syntax for consuming inputs independently like this, I haven't seen a concise way for looping over each of until both of the channels have closed.

for {
    select {
    case p, ok := <-mins:
        if ok {
            fmt.Println("Min:", p) //consume output
        }
    case p, ok := <-maxs:
        if ok {
            fmt.Println("Max:", p) //consume output
        }
    //default: //can't guarantee this won't happen while channels are open
    //    break //ideally I would leave the infinite loop
                //only when both channels are done
    }
}

the best I can think to do is the following (just sketched, may have compile errors):

for {
    minDone, maxDone := false, false
    select {
    case p, ok := <-mins:
        if ok {
            fmt.Println("Min:", p) //consume output
        } else {
            minDone = true
        }
    case p, ok := <-maxs:
        if ok {
            fmt.Println("Max:", p) //consume output
        } else {
            maxDone = true
        }
    }
    if (minDone && maxDone) {break}
}

But this looks like it would get untenable if you're working with more than two or three channels. The only other method I know of is to use a timout case in the switch statement, which will either be small enough to risk exiting early, or inject too much downtime into the final loop. Is there a better way to test for channels being within a select statement?

5 Answers 5

169

Your example solution would not work well. Once one of them closed, it would always be available for communication immediately. This means your goroutine will never yield and other channels may never be ready. You would effectively enter an endless loop. I posted an example to illustrate the effect here: http://play.golang.org/p/rOjdvnji49

So, how would I solve this problem? A nil channel is never ready for communication. So each time you run into a closed channel, you can nil that channel ensuring it is never selected again. Runable example here: http://play.golang.org/p/8lkV_Hffyj

for {
    select {
    case x, ok := <-ch:
        fmt.Println("ch1", x, ok)
        if !ok {
            ch = nil
        }
    case x, ok := <-ch2:
        fmt.Println("ch2", x, ok)
        if !ok {
            ch2 = nil
        }
    }

    if ch == nil && ch2 == nil {
        break
    }
}

As for being afraid of it becoming unwieldy, I don't think it will. It is very rare you have channels going to too many places at once. This would come up so rarely that my first suggestion is just to deal with it. A long if statement comparing 10 channels to nil is not the worst part of trying to deal with 10 channels in a select.

8
  • 2
    I would probably wrap that if statement into a local function. It would be less clutter and the function name would help make it more obvious what was happening (something along these lines: play.golang.org/p/347KZdI_Gs).
    – Chris
    Dec 2, 2012 at 5:50
  • 8
    +1 for the non-clever solution. It's important to set those channels to nil so that select doesn't waste time on them, but I might use a separate variable to count down the number of open channels. for n=2; n>0; { and then each time you set a channel to nil, n--.
    – Sonia
    Dec 2, 2012 at 7:31
  • 3
    From the language spec: "Since communication on nil channels can never proceed, a select with only nil channels and no default case blocks forever." Why doesn't the above block forever? [Edit: Oh wait yep the break will always happen before the first iteration of the loop where both are nil] Nov 20, 2015 at 4:59
  • 23
    I'd move the if into the for: for ch != nil || ch2 != nil {. Although this is one more check at the start, you'll never be in the endless loop, which @voutasaurus mentions (it still can happen if ch and ch2 were both nil at the start). And it looks cleaner in my opinion.
    – user
    Nov 24, 2015 at 21:18
  • It wouldn't loop forever just block forever but the outcome is similar. Nov 24, 2015 at 21:22
33

Close is nice in some situations, but not all. I wouldn't use it here. Instead I would just use a done channel:

for n := 2; n > 0; {
    select {
    case p := <-mins:
        fmt.Println("Min:", p)  //consume output
    case p := <-maxs:
        fmt.Println("Max:", p)  //consume output
    case <-done:
        n--
    }
}

Complete working example at the playground: http://play.golang.org/p/Cqd3lg435y

4
  • Nice solution! More idiomatic go than Stephen's solution IMHO. It is easy to get hung up on having to close channels so a reminder that you don't have to is very useful. Dec 2, 2012 at 9:07
  • 7
    Thank you, Nick. I wanted to show an alternative, but after sleeping on it, I think I Stephen's and jnml's solutions are more robust. For example if you make the seemingly simple change of buffering mins and maxs in my solution, you introduce a data race with the done channel and the program would drop whatever output happened to be in the buffer when the done value arrived. Also, I'm not strictly answering the question as asked and I'm speculating that the OP has control over the producer code. It's possible that he doesn't and that he must deal with a channel being closed as he described.
    – Sonia
    Dec 2, 2012 at 14:03
  • I think this might be more clear using a WaitGroup Feb 10, 2016 at 11:56
  • This is a good solution
    – manju4ever
    Nov 6, 2022 at 11:51
12

Why not use goroutines? As your channels are getting closed, the whole thing turns into a simple range loop.

func foo(c chan whatever, prefix s) {
        for v := range c {
                fmt.Println(prefix, v)
        }
}

// ...

go foo(mins, "Min:")
go foo(maxs, "Max:")
3
  • 1
    This was actually the first solution that came to mind for me. If you wanted the routine calling go foo to know when foo had completed, you would need to add synchronization for that. Also fmt.Println isn't thread safe and you can get occasional garbled output. log.Println is an easy alternative that is thread safe.
    – Sonia
    Dec 2, 2012 at 13:41
  • @Sonia, you also can use log.Println in a separate gorutine and ship messages to it. play ground.
    – Ivan Black
    Jan 12, 2016 at 22:10
  • @Sonia, I made the package. Boo-ha-ha-ha-ha.
    – Ivan Black
    Jan 13, 2016 at 0:49
3

When I came across such a need I took the below approach:

var wg sync.WaitGroup
wg.Add(2)

go func() {
  defer wg.Done()
  for p := range mins {
    fmt.Println("Min:", p) 
  }
}()

go func() {
  defer wg.Done()
  for p := range maxs {
    fmt.Println("Max:", p) 
  }
}()

wg.Wait()

I know this is not with single for select loop, but in this case I feel this is more readable without 'if' condition.

2
  • This does read more elegantly, but I don't believe fmt.Println is safe under concurrency—at least not for Stdout. That's actually why I was trying to join channels into a single switch, so that the actual program output occurred only on the main goroutine.
    – matthias
    May 10, 2019 at 17:16
  • 2
    That said, if you replace the fmt.Println calls with sends to a new shared channel output, then close output after wg.Wait() I think that would solve the safety issue. Then just loop over range output on main. That would enable the concise range syntax!
    – matthias
    May 10, 2019 at 17:19
2

I wrote a package which provides a function to solve this problem (among several others):

https://github.com/eapache/channels

https://godoc.org/github.com/eapache/channels

Check out the Multiplex function. It uses reflection to scale to an arbitrary number of input channels.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.