ChrisDave
ChrisDave

Reputation: 67

Golang Concurrency Issue

I am learning Golang concurrency and have written a program to display URL's in order. I expect the code to return
http://bing.com* http://google.com*

But it always returns http:/google.com*** . As if the variable is being overwritten.Since i am using goroutines i would expect it to return both values at the sametime.

func check(u string) string {
tmpres := u+"*****"
return tmpres
}

func IsReachable(url string) string {
ch := make(chan string, 1)
go func() { 

    ch <- check(url) 

     }()
select {
case reachable := <-ch:
    // use err and reply
    return reachable
case <-time.After(3* time.Second):
    // call timed out
    return "none"
}
   }



func main() {

var urls = []string{
  "http://bing.com/",
  "http://google.com/",
}

for _, url := range urls {
    go func() {
     fmt.Println(IsReachable(url)) 
     }()
}
time.Sleep(1 * time.Second)
  }

Upvotes: 1

Views: 449

Answers (2)

Vaishnav Pureddiwar
Vaishnav Pureddiwar

Reputation: 21

the above asnwer is very apt, will add some points on it.

  • 2 go-routines spunned by you refer the variable bing
  • the solution you have written can't be deterministic in nature i.e the first go-routine spunned will execute first it completely depends on the go-runtime
  • adding a time.Sleep is also a not good approach you should have went with sync.Waitgroup or the channel's one.

Upvotes: 1

Adrian
Adrian

Reputation: 46602

Two problems. First, you've created a race condition. By closing over the loop variable, you're sharing it between the thread running the loop and the thread running the goroutine, which is causing your described problem: by the time the goroutine that was started for the first URL tries to run, the value of the variable has changed. You need to either copy it to a local variable, or pass it as an argument, e.g.:

for _, url := range urls {
    go func(url string) {
     fmt.Println(IsReachable(url)) 
     }(url)
}

Second, you said you wanted to display them "in order", which is not a goal generally compatible with concurrency/parallism, because you cannot control the order of parallel operations. If you want them in order, you should do them in order in a single thread. Otherwise, you'll have to collect the results, wait for all them to come back, then sort the results back into the desired order before printing them.

Upvotes: 6

Related Questions