Cannot identify an error in sync.Once usage

Question

I'm doing an online course on Golang. The following piece of code is presented in the course material as an example of misuse of sync.Once:

var (
    once sync.Once
    db   *sql.DB
)

func DbOnce() (*sql.DB, error) {
    var err error
    once.Do(func() {
        fmt.Println("Am called")
        db, err = sql.Open("mysql", "root:test@tcp(127.0.0.1:3306)/test")
        if err != nil {
            return
        }
        err = db.Ping()
    })
    if err != nil {
        return nil, err
    }
    return db, nil
}

Supposedly, the above is a faulty implementation of an SQL connection manager. We, the students, are to find the error ourselves, which I struggle with. The code runs fine even in parallel. This is how I used it:

func main() {
    wg := sync.WaitGroup{}
    wg.Add(10)

    for i := 0; i < 10; i++ {
        go (func() {
            db, err := DbOnce()
            if err != nil {
                panic(err)
            }

            var v int
            r := db.QueryRow("SELECT 1")
            err = r.Scan(&v)
            fmt.Println(v, err)

            wg.Done()
        })()
    }

    wg.Wait()
}

I understand that homework questions are discouraged here, so I'm not asking for a complete solution, just a hint would be fine. Is the error related to concurrency (i.e. I need to run it in a specific concurrent context)? Is it usage of sql.Open specifically?

user_1705598 · Answer

Initialization of the db variable is OK. The problem is with the returned error.

If you call DbOnce() for the first time and opening a DB connection fails, that error will be returned properly. But what about subsequent calls? The db initialization code will not be run again, so nil db may be returned, and since the initialization code is not run, the default value of the err variable is returned, which will be nil. To sum it up, the initialization error is lost and will not be reported anymore.

One solution is to stop the app if connection fails (at the first call). Another option is to store the initialization error too in a package level variable along with db, and return that from DbOnce() (and not use a local variable for that). The former has the advantage that you don't have to handle errors returned from DbOnce(), as it doesn't even have to return an error (if there's an error, your app will terminate).

The latter could look like this:

var (
    once  sync.Once
    db    *sql.DB
    dbErr error
)

func DbOnce() (*sql.DB, error) {
    once.Do(func() {
        fmt.Println("Am called")
        db, dbErr = sql.Open("mysql", "root:test@tcp(127.0.0.1:3306)/test")
        if dbErr != nil {
            return
        }
        dbErr = db.Ping()
    })
    return db, dbErr
}

Cannot identify an error in sync.Once usage

1 Answers