go-sql-driver/mysql の QueryContext でコンテキストをキャンセルしたら race が起こる

タイトルの通り、 QueryContext の第一引数に渡した Context を、 Rows.Close() を呼び出す前にキャンセルすると、 race が起こる可能性があります。

修正する pull request を作成したのですが、メンテナが作ったプルリクエストは他のメンテナのレビューなしにマージしてはいけないというルールがあるのでまだマージしていません。なので、 QueryContext を利用するときは注意してください。

github.com

問題になるコード

Rows.Scan() の引数の型が *sql.RawBytes (sql.RawBytes の定義は type []byte RawBytes) だった場合、受け取った []byte の中身が他の goroutine から書き換えられることがあります。

rows, err := db.QueryContext(ctx, "select ...", ...)
if err != nil {
    // ...
}
defer rows.Close()

for rows.Next() {
    var sql.RawBytes
    err := rows.Scan(&buf)
    // どこかのタイミングで ctx がキャンセルされると、
    fmt.Println(buf) // buf の中身が他の goroutine から書き換えられる
}

背景

database/sql の設計は、 driver の API は1つの接続だけを扱う（コネクションプール部分は全て database/sql が解決する）、そしてその1つの接続に対して並行で driver の API が呼ばれることはない、ようになっています。

また、 rows.Scan() が sql.RawBytes を出力する場合、その参照先の部分は次の rows.Next() か rows.Close() 以降はドライバが再利用できるようになっています。これは受信バッファの中身を直接スライスで返すことで余分なアロケートとコピーを減らすためです。

コンテキスト対応が入るまではこれでうまくいっていました。しかし、 database/sql がコンテキストに対応する時に、コンテキストのキャンセルを別 goroutine で監視して、アプリケーションからの rows.Close() や rows.Scan() を待たずにドライバー側の rows.Close() を呼び出すようにしてしまったのです。

Go 1.10 の changelog

結果として次のような状況になりました。

rows.Scan() が返した sql.RawBytes (type []byte RawByte)は受信バッファの中を直接参照しています。アプリケーションは次の rows.Next() か rows.Close() を呼び出すまではその中身を合法的に読むことができます。
ドライバは rows.Close() や rows.Next() が呼ばれた場合、前回の Scan() で返した []byte の中身を破壊していいので、送受信バッファとして再利用します。
database/sql はコンテキストがキャンセルされると別 goroutine からドライバ側の rows.Close() を呼び出します。

正直、なんてことしてくれたんだ！って感じです。アプリケーションが rows.Next() を呼ぶまで待ってくれたらいいのに。なんでわざわざ別 goroutine で。。。

たぶん一刻も早くキャンセルしたかったんでしょうが、MySQLでは百害あって一理ありません。 MySQL でクエリのキャンセルは難しいのでまだ実装してないし、たとえ実装していたとしても、 rows.Next() が一回でも呼ばれているということはクエリの実行はすでに終わっています。 rows.Next() を待たずにキャンセルするメリットが現在も将来も一切ありません。

2019-04-03 追記: sql.RawBytes を使わなくても race が起こる

ユーザーコードに直接受信バッファの中身を参照する []byte が渡されるのは sql.RawBytes を利用したときだけです。

しかし、 database/sql の sql.Rows.Scan() は、 driver.Rows.Next() が返した値と Scan の引数の間の変換処理をしています (convertrows)。この変換処理は driver.Rows.Next() が返した受信バッファ内を直接参照する []byte から string を作ったり、新たな []byte を作ってそこにコピーしたりしています。

タイミング悪くこの変換処理中にコンテキストがキャンセルされた (厳密には database/sql 内の監視 goroutine がコンテキストのキャンセルを検出して driver.Rows.Close() を呼び出した) 場合は、ユーザーコードで sql.RawBytes を使っていなくても race が発生します。