Why does this golang function _not_ run forever?

2
votes

I wanted to try the FizzBuzz test (Why can't programmers program), and used Go. It's basically looping from 1 to 100, and printing "Fizz" when the loop counter is divisible by 3, "Buzz" when divisible by 5, "FizzBuzz" when divisible by both and else just print the number.

After doing it iteratively and recursively, I wanted to do it concurrently (or by using channels). I came up with the following code, which worked to my surprise:

func fizzbuzzconc() {
    // Channels for communication
    fizzchan := make(chan int)
    buzzchan := make(chan int)
    fizzbuzzchan := make(chan int)
    nonechan := make(chan int)

    // Start go routine to calculate fizzbuzz challenge
    go func() {
        for i := 1; i <= 100; i++ {
            if i % 3 == 0 && i % 5 == 0 {
                fizzbuzzchan <- i
            } else if i % 3 == 0 {
                fizzchan <- i
            } else if i % 5 == 0 {
                buzzchan <- i
            } else {
                nonechan <- i
            }
        }
    }()

    // When or how does this for loop end?
    for {
        select {
        case i := <-fizzchan:
            fmt.Println(i, "Fizz")
        case i := <-buzzchan:
            fmt.Println(i, "Buzz")
        case i := <-fizzbuzzchan:
            fmt.Println(i, "FizzBuzz")
        case i  := <-nonechan:
            fmt.Println(i, i)
        }
    }
}

I can't understand how and why the for loop stops. There is no break condition, or a return statement. Why does it eventually finish running?

3
gochannelfizzbuzz
Add a default to your select. Any change? - bishop
When I add a default the execution gets much slower, and it doesn't exit. Now I have the expected behaviour, thanks. - Kiril
Yep. The reason that happens is that all your case enter a system call via Println. Go checks channel state during system calls and sees, when the last one empties, that all the channels are done. At that point, it's deadlock detector kicks in and kills the loop. When you add an empty default, there's a branch that doesn't go through system calls and go just spins through it. The smallest change you can make to get your code to exit cleanly is to add a wait group channel close in your default case, as seen in this answer: stackoverflow.com/a/21819794/2908724 - bishop

3 Answers

1
votes

It doesn't really work well.

What happens, after a time, is that there's an attrition due to the remaining go-routine waiting for a channel where no goroutine pushes. So what you have is a dead-lock (which is a fatal error ending the program), not a clean ending.

In summary it "works" because the go engine is smart enough to detect the dead lock.

0
votes

@dystroy has answered your question very well, however here is how you might fix your code.

One way of exiting tidily is using a quit channel which we signal by closing it. (Signalling by closing a channel is useful because more than one go routine can be listening on it at once).

There are other ways of doing this though - if you only have one output channel then you range over it to read the result and close it when you finished. You could easily re-write this to work that way.

You can use a sync.Waitgroup to make sure the go routine has finished also.

Playground

func main() {
    // Channels for communication
    fizzchan := make(chan int)
    buzzchan := make(chan int)
    fizzbuzzchan := make(chan int)
    nonechan := make(chan int)
    quit := make(chan struct{})

    // Start go routine to calculate fizzbuzz challenge
    go func() {
        for i := 1; i <= 100; i++ {
            if i%3 == 0 && i%5 == 0 {
                fizzbuzzchan <- i
            } else if i%3 == 0 {
                fizzchan <- i
            } else if i%5 == 0 {
                buzzchan <- i
            } else {
                nonechan <- i
            }
        }
        close(quit)
    }()

    // When or how does this for loop end?
OUTER:
    for {
        select {
        case i := <-fizzchan:
            fmt.Println(i, "Fizz")
        case i := <-buzzchan:
            fmt.Println(i, "Buzz")
        case i := <-fizzbuzzchan:
            fmt.Println(i, "FizzBuzz")
        case i := <-nonechan:
            fmt.Println(i, i)
        case <-quit:
            break OUTER
        }
    }
    fmt.Println("All done")
}
0
votes

@OneOfOne mentioned the sync.WaitGroup method which I think falls most in line with how you would do this in Go. Considering goroutines are very cheap and the problem can be solved in parallel, we can create a go routine for each input and send the results on a buffered channel.

//Size to check
size := 100

results := make(chan Result, size)

// Create the WaitGroup and set the latch size.
var wg sync.WaitGroup
wg.Add(size)

// Create a goroutine for each parallel operation
for i := 1; i <= size; i++ {
    i := i  //bind value into closure
    go func() {
        results <- fizzbuzz(i)
        wg.Done()  //release a latch
    }()
}

//wait for all the goroutines to finish.
wg.Wait()

//close the channel so we can exit the below for loop.
close(results)

//Range over the results and exit once the channel has closed.
for x := range results {
    fmt.Printf("i: %d result: %s\n", x.Nr, x.Val)
}

Playground code: http://play.golang.org/p/80UafMax7M