gammazero / workerpool

Concurrency limiting goroutine pool
MIT License
1.33k stars 138 forks source link

Bug - strange behavior #69

Open surjit opened 1 year ago

surjit commented 1 year ago

Ubuntu 20.04 LTS go version go1.19.3 linux/amd64 github.com/gammazero/workerpool v1.1.3

package main

import (
    "fmt"
    "github.com/gammazero/workerpool"
)

type JobAccount struct {
    Id       int
    Username string
}

func main() {
    list := []string{
        "demo1", "demo2", "demo3",
    }

    var accounts []*JobAccount

    for k, v := range list {
        accounts = append(accounts, &JobAccount{
            Id:       k,
            Username: v,
        })
    }

    wp := workerpool.New(2)

    for _, account := range accounts {
        fmt.Printf("loop %+v\n", account.Username)

        wp.Submit(func() {
            fmt.Printf("go rountine %+v\n", account.Username)
        })
    }

    wp.StopWait()
}

Here actual output

loop demo1
go rountine demo1
loop demo2
loop demo3
go rountine demo3
go rountine demo3

I was expecting

loop demo1
go rountine demo1
loop demo2
loop demo3
go rountine demo2
go rountine demo3
hut8 commented 1 year ago

This isn't a bug in workerpool, it's how go works.

    for _, account := range accounts { // <- here you are assigning a pointer to a variable named account
        fmt.Printf("loop %+v\n", account.Username) // <- you'll always get the expected answer here

        wp.Submit(func() {
                        // here, you are capturing the outer pointer to "account"
                        // by the time this function is actually run, it may have a different value. That's your problem.
            fmt.Printf("go rountine %+v\n", account.Username)
        })
    }

This is actually clearly documented right in the sample:

    for _, r := range requests {
        r := r // <-- 🙂
        wp.Submit(func() {
            fmt.Println("Handling request:", r)
        })
    }

See also http://devs.cloudimmunity.com/gotchas-and-common-mistakes-in-go-golang/index.html#closure_for_it_vars

See also https://github.com/gammazero/workerpool/issues/67