Go concurrency, goroutine synchronization and closing channels-CodePudding

Familiarizing myself with concurrency, so started writing a simple ping cli with concurrent calls (let's ignore that I'm not really measuring pings).

Problem is, I can't close the channel properly for the range loop while also waiting for all the goroutines to finish. If I want to concurrently call the ping function, I can't synchronize it with waitgroups, because I will reach the wg.Wait() line before all the goroutines finish.

Is there a way to keep the ping calls concurrent and close the channel after they're done, so the range loop can exit?

func main() {
    domain := flag.String("domain", "google.com", "the domain u want to ping")
    flag.Parse()

    sum := 0
    ch := make(chan int)

    go start_pings(*domain, ch)

    for elapsed := range ch {
        fmt.Println("Part time: "   strconv.Itoa(elapsed))
        sum  = elapsed
    }

    avg := sum / 3
    fmt.Println("Average: "   strconv.Itoa(avg))
}

func start_pings(domain string, ch chan int) {
    var wg sync.WaitGroup
    for i := 0; i < 3; i   {
        wg.Add(1)
        go ping(domain, ch, wg)
    }
    wg.Wait()
    close(ch)
}

func ping(domain string, ch chan int, wg sync.WaitGroup) {
    url := "http://"   domain
    start := time.Now()

    fmt.Println("Start pinging "   url   "...")

    resp, err := http.Get(url)
    elapsed := time.Now().Sub(start)

    if err != nil {
        fmt.Println(err)
        return
    }
    defer resp.Body.Close()

    ch <- int(elapsed)
    wg.Done()
}

CodePudding user response：

You must not copy a sync.WaitGroup! It's doc explicitly states:

A WaitGroup must not be copied after first use.

Pass a pointer to it: wg *sync.WaitGroup. And call wg.Done() deferred! You have other return statements which will cause wg.Done() to be skipped!

func start_pings(domain string, ch chan int) {
    var wg sync.WaitGroup
    for i := 0; i < 3; i   {
        wg.Add(1)
        go ping(domain, ch, &wg)
    }
    wg.Wait()
    close(ch)
}

func ping(domain string, ch chan int, wg *sync.WaitGroup) {
    defer wg.Done()
    url := "http://"   domain
    start := time.Now()

    fmt.Println("Start pinging "   url   "...")

    resp, err := http.Get(url)
    elapsed := time.Since(start)

    if err != nil {
        fmt.Println(err)
        return
    }
    defer resp.Body.Close()

    ch <- int(elapsed)
}

Today's IDE's (and golinter) warns about such obvious misuses. To avoid such mistakes, declare wg to be a pointer in the first place:

func start_pings(domain string, ch chan int) {
    wg := &sync.WaitGroup{} // Pointer!
    for i := 0; i < 3; i   {
        wg.Add(1)
        go ping(domain, ch, wg)
    }
    wg.Wait()
    close(ch)
}

This leaves less room for errors and misuses.