去例行公事

Question

I am leaning go, so this might be a stupid question.

I can't seem to figure out why one of my go routines is being blocked by another. My understanding (could be wrong) go routines run as independent light weight threads so they should not be blocking each other, unless I have messed up :)

I have pasted the code below and would appreciate any help / tips to figure this out.

package main
import "fmt"
import "time"
import "sync"



func worker( jobs <-chan int, job2 chan<- int) {
    defer wg.Done()
    for j := range jobs {
        fmt.Println("finished job", j)
        time.Sleep(time.Second/2)

        if(j%3==0){
           job2 <- j   
        }  

   } 
   close(job2)
   fmt.Println("channel job2 closed")
 }

func worker2(job2 <-chan int) {
    defer wg.Done()
    for i:= range job2 {
        fmt.Println(i)
        time.Sleep(time.Second*10)
    } 
}

var wg sync.WaitGroup

func main() {

    wg.Add(2)
    jobs := make(chan int)

    job2 := make(chan int)

    go func() {
        for j := 1; j <= 10; j++ {
             jobs <- j
        }
        close(jobs)
        fmt.Println("channel jobs closed")
    }()

    go worker(jobs,job2)
    go worker2(job2)

    wg.Wait()
    fmt.Println("exiting main")     

}

I get the following output when I run this code

finished job 1
finished job 2
finished job 3
finished job 4
3
finished job 5
finished job 6
6
finished job 7
finished job 8
finished job 9
9
finished job 10
channel jobs closed
channel job2 closed
exiting main

however I was expecting something like this?

finished job 1
finished job 2
finished job 3
finished job 4
3
finished job 5
finished job 6
finished job 7
finished job 8
finished job 9
finished job 10
channel jobs closed
6
9    
channel job2 closed
exiting main

Answer 1

Your routines are sort-of blocking because the channels aren't buffered. A write/read on an unbuffered channel is a blocking operation. Therefore your routines by definition have to wait on eachother.

Essentially, your sleep of half a second is kind of irrelevant, because the second worker sleeps for 10 seconds. Those 10 seconds will block reads/writes to the second channel. Add a buffer to the channel to get around this.

Some other things I'd like to point out are:

• time.Sleep(time.Second/2) isn't going to work (well, it is, but dividing by 3 for example isn't). time.Sleep expects a time.Duration argument, which is an int64. You need to pass something like time.Millisecond * 500 instead
• It's bad form to pass a channel to a routine, and close it from the routine that didn't create the channel. The channels creation and closing should be contained within a single routine. If not, it works, but maintenance becomes a real nightmare.
• Group your imports, instead of repeating the import "package", just use import ( "package1" "package2")
• Don't use globals if you don't have to. Create the waitgroup in the function that starts all of the routines, and pass a pointer to it to all routines. Including the anonymous function, just to be safe (once you start adding buffers to channels, for example)
• Consider looking into context.Context and the select construct. You can create a context.WithCancel and in select listen for ctx.Done() in all routines. Then you can just cancel all routines in one go without having to handle signals and pushing stuff onto a cancel channel

Demo

I've changed a couple of things (mainly channel creation, and some minor code-cleanup), and created a playground example here