Data race in sql package

[Michal Hruby]

Hello, I have a code snippet equivalent to this one:

rows, err := stmt.QueryContext(ctx)
if err != nil {
    return info, nil, err
}
defer rows.Close()

var data sql.RawBytes
for rows.Next() {
    err = rows.Scan(&data)
    if err != nil {
        return nil, err
    }
    // if ctx is canceled around this point, rows.Close() will be called and the data can e overwritten, causing a data race
    _ = data[0] // <--- possible concurrent read here and write inside driver.Rows.Close()
}

And as the comments say, if the context is cancelled, there's a possibility of a data race where this goroutine is trying to read data[0], and another goroutine can be writing to it, cause the sql package calls driver.Rows.Close() (from https://github.com/golang/go/blob/176b63e7113b82c140a4ecb2620024526c2c42e3/src/database/sql/sql.go#L2974 )

I've opened an issue about this on github ( https://github.com/golang/go/issues/53970 ), but was told that's not the proper forum to talk about data races.

Any help would be appreciated, thanks.

[Ian Lance Taylor]

I don't quite understand what you are asking. I think you're correct
that there is a data race there. It seems to me that the answer is to
not cancel the context.

What kind of answer are you looking for?

Ian

[Steven Hartland]

I'm guessing that Michal is flagging is there no way to write safe code if your using stmt.QueryContext(ctx) and rows.Scan into a sql.RawBytes as QueryContext kicks off a go routine that monitors the ctx, closing rows if cancelled and that ctx can be cancelled at any time.

The only thing that springs to mind is to only allow the cancellation processing to happen while in sql functions which would mean that sql.RawBytes can't be updated while the consumer, user code, is processing it.

It's interesting that there is a related comment in row.Scan, which prevents the use of sql.RawBytes because it always closes the underlying rows before returning.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAOyqgcUwGN5b-DEwCqLRu6kWwpAnCvO7o6u5TjzS3bmbUgksdw%40mail.gmail.com.

[Michal Hruby]

That's right, atm there's no safe way to use sql.RawBytes, it should either be documented that you need to manually control the context, or the standard library shouldn't be closing the driver.Rows unless the user calls Scan or Close.

[Daniel Theophanes]

It should be safe to use RawBytes from *Conn or *Tx, the transaction won't be re-used. We will look into it further.