Go routine returns less results then the actual result-CodePudding

I have a LOOP that does the hashing inside for a given key and returns the result, but on Result, If I have 1500 list of URL that goes inside LOOP, it never returns the result of 1500, it returns always lesser then 1500.

What am I doing wrong in below:

if len(URLLists) > 0 {
    var base = "https://example.com/query?="
    var wg sync.WaitGroup
    var mutex = sync.Mutex{}
    wg.Add(len(URLLists))
    for _, url := range URLLists {
        // wg.Add(1)  OR above for loop
        go func() {
            defer wg.Done()
            hmac := "HMAX_123"
            out := encoding.HexEncodeURL(hmac, url)
            final := base   out
            list := Lists{
                Old: url,
                New: final,
            }
            mutex.Lock()
            response.URL = append(response.URL, list)
            mutex.Unlock()
        }()
    }
    wg.Wait()
    jR, err := json.Marshal(response)
    if err != nil {
        w.Write([]byte(`{"success": false, "url" : ""}`))
    } else {
        w.Write(jR)
    }
    return
}

I tried both method for Add - one inside loop by 1 and one outside loop by total length.

I want function to return all 1500 URL list and not just "700, 977, 1123" random list.

It looks like - wg.Wait() is not waiting for all the wg.Add - added

CodePudding user response：

You have a pretty serious race condition here:

response.URL = append(response.URL, list)

If you're starting as many as 1500 concurrent Go routines, you will have many hundreds of them all attempting to execute this line at once. They will be constantly overwriting changes to the array.

You need to protect the insertion of new data into this slice with a sync.Mutex, or be sending results over a channel and having a single Go routine reading from that channel and appending to the list.

CodePudding user response：

There are a couple of errors in this program:

You are using the loop variable inside the goroutine. The loop variable is rewritten at each iteration, so when the goroutine uses the url, it might have already moved on to the next URL, thus you end up with multiple goroutines hashing the same URL. To fix:

    for _, url := range URLLists {
        url:=url // Create a copy of the url
        // wg.Add(1)  OR above for loop

You have a race condition. You have to protect access to response.URL because it is being written by multiple goroutines. You can use a mutex:

lock:=sync.Mutex{}
for _,url:=...
  ...
  lock.Lock()
  response.URL = append(response.URL, list)
  lock.Unlock()

A better way would be to send these over a channel.