methaneのブログ

このブログに乗せているサンプルコードはすべてNYSLです。

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

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

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

github.com

問題になるコード

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

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

for rows.Next() {
    var []buf
    err := rows.Scan(&buf)
    fmt.Println(buf) // buf の中身が他の goroutine から書き換えられる
}

また引数の型が *interface{} だった場合にも、その interface{}[]byte が格納され、その中身が他の goroutine から書き換えられることがあります。

背景

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

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

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

Go 1.10 の changelog

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

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

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

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