Golang Concurrency Issue

Question

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)
  }

tkausl · Accepted Answer · 2017-08-17 17:05:12Z

6

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.

edited Aug 17, 2017 at 17:05

tkausl

14.4k3 gold badges37 silver badges55 bronze badges

answered Aug 17, 2017 at 17:02

Adrian

47.2k6 gold badges121 silver badges110 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

ChrisDave Over a year ago

Thanks Adrian for the reply, it solved the problem. Instead of ordering i will concatenate the strings, this will help rest of my program. Iam currently doing it by taking values into channel, test := IsReachable(url) message <- test and then at the end using a for-loop. Is there a better way to do the same? for i := 1; i <= len(urls); i++ { bufferstr = append(bufferstr,<-message) } Its working well , just curious to know if this is optimum programming.Appreciate the help

Vaishnav Pureddiwar · Accepted Answer · 2024-07-30 13:33:10Z

1

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.

answered Jul 30, 2024 at 13:33

Vaishnav Pureddiwar

212 bronze badges

Collectives™ on Stack Overflow

Golang Concurrency Issue

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related