Missing data in the channel

Question

I wrote a small program to practice go channel.

package main

import (
    "log"
    "strconv"
)

var MaxOutstanding int = 1
var channelSize int = 10
var sem = make(chan int, MaxOutstanding)

type Request struct {
    command string
    data    string
}

func process(req *Request) {
    log.Println(req)
}

func serve(reqs chan *Request) {
    for req := range reqs {
        sem <- 1
        go func() {
            process(req)
            <-sem
        }()
    }
}

func main() {
    reqs := make(chan *Request, channelSize)
    for i := 0; i < channelSize; i++ {
        req := &Request{"start", strconv.Itoa(i)}
        reqs <- req
    }
    close(reqs)
    serve(reqs)
}

This prints

2018/12/02 16:52:30 &{start 1}
2018/12/02 16:52:30 &{start 2}
2018/12/02 16:52:30 &{start 3}
2018/12/02 16:52:30 &{start 4}
2018/12/02 16:52:30 &{start 5}
2018/12/02 16:52:30 &{start 6}
2018/12/02 16:52:30 &{start 7}
2018/12/02 16:52:30 &{start 8}
2018/12/02 16:52:30 &{start 9}

Therefore, &{start 0} is not printed. Why is this missing?

Your thing with `sem` is weird. If you wanted a limited number of readers on a channel, then only launch that many readers. Each reader would read `reqs`. Eh, whatever. Your `sem` thing is just not like other Go code I've read. — Zan Lynx, Dec 03 '18 at 01:14
Anyway this is how I would write it, not that you have to do it this way: https://play.golang.org/p/RB3sUoSaSxm — Zan Lynx, Dec 03 '18 at 01:27

icza · Accepted Answer · 2018-12-03T08:52:21.043

4

Because in serve() your loop variable is used inside the function literal you execute on a separate goroutine, which is concurrently modified by the goroutine running the loop: data race. If you have data race, the behavior is undefined.

If you make a copy of the variable, it will work:

for req := range reqs {
    sem <- 1
    req2 := req
    go func() {
        process(req2)
        <-sem
    }()
}

Try it on the Go Playground.

Another possibility is to pass this to the anonymous function as a parameter:

for req := range reqs {
    sem <- 1
    go func(req *Request) {
        process(req)
        <-sem
    }(req)
}

Try this one on the Go Playground.

This is detailed in several related questions:

Using Pointers in a for Loop - Golang

Golang: Register multiple routes using range for loop slices/map

Why do these two for loop variations give me different behavior?

Also as Zan Lynx noted, your main goroutine does not wait for all launched goroutines to complete, so you may not see all the requests printed. See this question how you can wait started goroutines: Prevent the main() function from terminating before goroutines finish in Golang

edited Dec 03 '18 at 08:52

answered Dec 03 '18 at 01:10

icza

389,944
63
907
827

I noticed that yours (and the original code) doesn't do 9 because it doesn't wait long enough for the goroutines to complete. – Zan Lynx Dec 03 '18 at 01:33
@ZanLynx Yes, you're right, I was only focusing on the problem in question. – icza Dec 03 '18 at 01:37
I realize these mistakes are also discussed in the "effective go" article. However, after reading the article, i still did not understand how the second suggestion work. I.e., req is a pointer and the for loop is still changing it which is the same situation in my code. The only difference is the req is passed as an argument in the closure. Could you elaborate how it work? – drdot Dec 03 '18 at 06:39
@drdot Added 3 links to related questions where it is detailed. – icza Dec 03 '18 at 08:52

score 1 · Answer 2 · answered Dec 03 '18 at 04:33

This is because of when the anonymous execuate, the req is alreay has move from Request{0} to Request{1}, so you print from {start 1}.

// method 1: add a parameter, pass it to anonymous function
func serve(reqs chan *Request) {
    for req := range reqs {
        sem <- 1

        // You should make the req as parameter of this function
        // or, when the function execute, the req point to Request{1}
        go func(dup *Request) {
            process(dup)
            <-sem
        }(req)
    }

    // And you should wait for the Request{9} to be processed
    time.Sleep(time.Second)
}

// method 2: make for wait anonymous function
func serve(reqs chan *Request) {
    for req := range reqs {
        go func() {
            process(req)
            sem <- 1
        }()

        // Or wait here
        <-sem
    }
}

Missing data in the channel

2 Answers2