Created
October 19, 2015 13:13
-
-
Save michaljemala/73dde464b6b78becf2d1 to your computer and use it in GitHub Desktop.
Fix for the http://www.gofragments.net/client/blog/concurrency/2015/09/22/goroutinesSyncWithWaitGroup/index.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
package main | |
import ( | |
"fmt" | |
"io/ioutil" | |
"log" | |
"net/http" | |
"sync" | |
) | |
func main() { | |
urls := []string{ | |
"http://www.reddit.com/r/programming.go", | |
} | |
// a channel | |
jsonResponses := make(chan string) | |
var wg sync.WaitGroup //def.1 | |
wg.Add(len(urls)) //def.2 | |
for _, url := range urls { | |
// as many goroutines as there are urls ! | |
// Note the way each goroutine gets a different url value, | |
// through the use of an "anonymous closure". | |
// The current url is passed to the closure as a parameter, | |
// masking the url variable from the outer for-loop. | |
// This allows each goroutine to have its own copy of url; | |
// otherwise, the next iteration of the for-loop would update url in all | |
// goroutines. | |
go func(url string) { | |
defer wg.Done() //def.3, inside each Goroutines | |
res, err := http.Get(url) | |
if err != nil { | |
log.Fatal(err) | |
} else { | |
defer res.Body.Close() | |
body, err := ioutil.ReadAll(res.Body) | |
if err != nil { | |
log.Fatal(err) | |
} else { | |
jsonResponses <- string(body) //send on channel | |
} | |
} | |
}(url) | |
} | |
go func() { | |
wg.Wait() //def.4 | |
// finished... close the channel | |
close(jsonResponses) | |
}() | |
for response := range jsonResponses { //receive on channel until we consumed everything | |
fmt.Println(response) | |
} | |
} | |
/* Expected Output: | |
A list of .... text in json format. | |
*/ |
Hello Patrick, the suggested solution of yours is still flawed. Should there be more then 1 url, it may easily happen that the quickest routine will close the jsonResponses
channel before the others could have chance to write their responses, thus causing the program to crash (send to a closed channel will cause panic).
Correct. Version 3... to come... and maybe version 4...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Thanks for your review of this fragment.
(Looking forward to your feed-back, read the modification I plan to introduce gofragments.net).
I admit there is a flaw because "ideally the goroutine responsible for sending on a channel should also be responsible for closing it" and the range will work until the channel is closed explicitly. And obviously, the place where the channel was closed in my initial version was somewhat "weird". The go func() { wg.Wait() close(channel)} idiom was initially the one I was considering but I modified it to give a simpler form, keeping the focus on the WaitGroup sequencing. If you have a more decisive reason to prefer the "voiture balai" (in french it refers to the Tour de France car that grabs the "out of time" cyclists...) approach (promoted by D.Cheney) let me know it. At present, I think the modification I will introduce will be the following (it'll keep the print in a goroutine/background).
/*
Usin sync.WaitGroup, an essential idiom to manage well the synchronization of
goroutines. See "def" 1 to 4 in this example: the WaitGroup idiom.
Package sync provides basic synchronization primitives
such as mutual exclusion locks.
Other than the Once and WaitGroup types, most are intended for use
by low-level library routines.
Higher-level synchronization is better done via channels and communication.
*/
package main
import (
"fmt"
"io/ioutil"
"log"
"net/http"
"sync"
)
func main() {
urls := []string{
"http://www.reddit.com/r/programming.go",
}
//a channel
jsonResponses := make(chan string)
}
/* Expected Output
A list of .... text in json format.
*/
And I will add the "voiture balai" idiom you bring forward in the comment.