前回のあらすじ
MDEV-24582に取り組むうちに、GDBが仲間に加わった。コードを実行しながら変数の中身も見えるすごいやつである。強力な仲間たちとともに、今日もIssueに立ち向かう。
とはいえ、とりあえず気持ちを変えてみる
ここ最近取り組んでいるMDEV-24582はなかなかの難敵なので、修正方針についてメンテナに合意を得てから実装に着手することにしよう。修正を追えてPRを出したあとに根本的な指摘をされるのはお互いに避けたいだろう。
該当のチケットに方針に関するたたき台を投稿したので、しばし返信を待つ。そしてこのままではブログネタがなくなるので、今日は別のIssueに取り組むことにする。
今回のIssue
特定のクエリを実行した際に、MariaDBサーバーがクラッシュするというバグ。クラッシュした際のスタックトレースが添付されているので、これを参考に見ていこう。
まずは例によって、問題が再現するクエリを書いたテストケースを作成し、実行してみる。
# mysql-test/suite/innodb/t/MDEV-18873.test ALTER TABLE t ADD PERIOD IF NOT EXISTS FOR `` (s,e); # run test ./mysql-test-run.pl innodb.MDEV-18873 --manual-gdb
ADD PERIOD という命令は初見だと思ったら、MariaDB特有の機能らしい。正直ほぼMySQL == MariaDBだと思っていたのだが、まれによく相違点があって面白い。
原因の特定
Issueのスタックトレースから Compare_identifiers::operator() でエラーが起きていることが分かるので、そこにブレークポイントを配置する。
#3 <signal handler called> #4 0x000056225dec77ca in Compare_identifiers::operator() (this=0x7fd1a807a41f, a=..., b=...) at /data/src/10.4/sql/vers_string.h:42 #5 0x000056225ded034f in Lex_cstring_with_compare<Compare_identifiers>::streq (this=0x7fd198005e80, b=...) at /data/src/10.4/sql/vers_string.h:91 #6 0x000056225e0ec019 in LEX::add_period (this=0x7fd198004960, name=..., start=..., end=...) at /data/src/10.4/sql/sql_lex.h:4363 ...
struct Compare_identifiers { int operator()(const LEX_CSTRING& a, const LEX_CSTRING& b) const { DBUG_ASSERT(a.str[a.length] == 0); DBUG_ASSERT(b.str[b.length] == 0); return my_strcasecmp(system_charset_info, a.str, b.str); } };
デバッガで追うと、 DBUG_ASSERT(a.str[a.length] == 0); でエラーが発生することが分かる。DebugビルドとReleaseビルドで挙動が異なるのも、このためだろう。今回のクエリでは、おそらくテーブル t が存在しないために、 a がNULLポインタになる模様。このため、 a.str がSegmatation faultとなり、サーバーがクラッシュする。
Releaseビルドでは、 my_strcasecmp の中でエラーが発生するが、これも a がNULLであることに起因するようなので、同じ原因と言えるだろう。
対処方法 - 1st trial
a.str を参照する前に、 a がNULLかどうか確認するのが最低限の対応となるだろう。以下のような判定を追加した。これまでは str がNULLの場合は必ず実行時エラーになっていたはずのため、この判定を追加しても既存の動作に悪影響はないはずだ。
bool streq(const Lex_cstring_with_compare& b) const
{
- return Lex_cstring::length == b.length && 0 == Compare()(*this, b);
+ return Lex_cstring::length == b.length && str != NULL && 0 == Compare()(*this, b);
}
やや対症療法的な感じはするが、シンプルで機能する修正ではあり、実際今回のエラーは解消する。一旦PRを作成して、レビュワーに確認してみることにする。
レビュー
半日程度でレビューが返ってきた。早い。実は今回のレビュワーは全く面識のないメンテナである。この方の視点では、謎の日本人からパッチが送られてきても、修正が妥当なのか不安に思うだろう。レビューのコメントも、修正の妥当性を説明することを求めているようだ。
たしかに言われてみれば、修正してはみたもののよくわからない点も多々あることに気付かされた。グローバルの強い開発者にコメントをもらえるのはOSSコントリビューションの醍醐味の一つと言える。その中で自身の理解の解像度を高めていけるのだから、これはとても良いことだ。
時間を割いてくれたレビュワーに感謝、不完全なPRを出した自分を陳謝しつつ、再度調査をしてみよう。
次回に続く。
