BoaのArray.fromでiterator関数をrootし忘れ、GC後にSIGSEGVになった

OmoikaneのAcid3を動かしていたら、macOS ARM64のbaseline JIT構成でプロセスがSIGSEGVになった。前に書いたOmoikaneの更新記事では短く触れたが、原因はテストそのものでもJITの機械語生成でもなく、取り込んでいるBoaのArray.from()にあった。Omoikaneはブラウザ側の実装とともにBoaのソースを取り込んでいるので、修正もその取り込み先のエンジンに入れている。

Symbol.iteratorのgetterから受け取った関数を、途中でGCが走っても使えるように保持していなかった。修正はroot()を呼ぶ1行だが、そこへたどり着くまでの切り分けと、修正前に確実に失敗するテストを作る過程が面白かったので、別の記事として残しておく。これは以前扱ったPromiseやshared shapeの問題とは別件。調査の記録はOmoikane Issue #777にある。

Acid3の失敗を時間切れと分ける

発見したのは、フォーム関連機能を追加するPR #775のBrowser CIだった。Acid3にはページ本来のイベントとtimerで進めるFaithfulと、テスト側から更新処理を駆動するDirectDriveの二つの経路がある。Faithfulはruntimeの破棄と強制GCまで終わっていたが、その後のDirectDrive中にmacOS ARM64のbaseline JIT構成だけがsignal 11で停止した。

この二つの経路を分けて見ないと、単に「Acid3で落ちた」としか言えない。Faithfulが終わったという記録は、テスト全体が正常に終わったという意味ではなく、次に実行されたDirectDriveの途中で別の経路へ入ったことを示している。SIGSEGVはOSが不正なメモリアクセスを検出した結果で、JavaScriptの例外やテストのスコア低下とも違う。

同じAcid3では別に時間切れも調べていた。しかし、制限時間に到達したケースと、プロセスが不正なメモリアクセスで落ちたケースは分ける必要がある。失敗したCIを1回再実行すると、同じDirectDrive中にSIGSEGVが再現した。一方、Linux ARM64の同じ構成でAcid3を4回実行した結果はすべて成功した。ここまでではmacOS固有とも、JIT固有とも言えない。

GCの寿命問題なら、入力が同じでも回収が起こる場所や、その時点でほかに残っている参照によって表面化の仕方が変わる。Linuxで4回成功した事実は重要だが、それだけでBoaの参照保持を疑いから外す理由にはならない。逆に、macOSで再現したことだけでJITの機械語が間違っているとも言えない。ここでは「どこで止まったか」を取る必要があった。

そこで、変更前後のrevisionを固定したmacOS診断CIを用意した。mainと修正前PRのrevisionは通常実行4回、LLDB付き4回のすべてで成功し、問題を観測したPR revisionは通常実行とLLDB付き実行の両方で落ちた。LLDBで見えた停止経路は次のとおり。

診断では、同じ環境でソースの組み合わせを変え、通常実行とデバッガ付き実行を別々に記録している。デバッガを付けるとタイミングやメモリ配置が変わるため、LLDBでだけ成功しても安心できない。今回の問題を観測したrevisionは、両方で異常終了した。

Array::from
  -> get_iterator_from_method
  -> JsObject::call
  -> __call__

iterator用関数のvtableを参照するところで停止していた。これは原因そのものを示すログではないが、Acid3全体ではなくArray.from()の関数呼び出し周辺に調査範囲を狭められる。なお、PRの変更がバグを新たに作ったとまでは、このrevision比較だけでは言えない。処理の順序やGCの起こるタイミングが変わり、以前からあった寿命の穴が見えるようになった可能性もある。

vtable参照で落ちたというだけなら、関数オブジェクト自体が壊れたのか、その前に別のメモリ破壊があったのかはまだ分からない。ただ、GetMethodで取得した関数が、呼び出されるまでどこに保持されるかを調べる手掛かりにはなった。ここからArray.from()の実装と仕様の順番を見直した。

`Array.from()`は関数を取得してから出力を作る

ECMAScriptのArray.from()の手順を見ると、iterableを扱う経路は大まかにこう進む。

itemsからSymbol.iteratorのメソッドを取得
  -> 出力先のconstructorを実行、または配列を生成
  -> 取得済みのメソッドを呼んでiteratorを得る

先にメソッドを取得し、その関数をまだ呼ばないうちに出力先を生成するのがポイントだ。Array.from.call(Destination, items)なら、Destinationのconstructorはこの隙間で実行される。JavaScriptのconstructorは任意の処理を行えるので、そこでGCが走ることもある。実装側は、先ほど取得したメソッドをその間ずっと生かしておかなければならない。

この順番は実装の都合ではなく、JavaScriptから見える。例えば次のコードではgetter、constructor、iterator関数の呼び出しがそれぞれログに残る。これは標準の動作順を確かめる例であって、GCのバグを再現するコードではない。

const order = [];
const source = {
  get [Symbol.iterator]() {
    order.push("get");
    return function () {
      order.push("call");
      return [37][Symbol.iterator]();
    };
  },
};
function Destination() {
  order.push("construct");
}

const result = Array.from.call(Destination, source);
console.log(order); // ["get", "construct", "call"]

取得したiterator関数をconstructorの前に呼ぶよう実装を並べ替えれば、get、call、constructの順になってしまう。constructorが例外を投げる場合、元の順序ではiterator関数は呼ばれないはずだが、並べ替えると呼ばれてしまう。したがって、途中にGCの危険があるからといって呼び出し順を変えるのは修正にならない。

普通はitems[Symbol.iterator]がオブジェクト上の関数を指しているため、itemsをたどれば関数へ到達できることが多い。だがgetterなら話が変わる。getterが毎回新しい関数を作って返しても、その関数をitemsのプロパティとして保存する必要はない。

仕様のGetMethodは、Symbol.iteratorというキーから得た値が呼び出し可能かを確かめる。そのプロパティがgetterなら、値の取得そのものがJavaScriptコードを実行する。Boaの実装も、プロパティを取得し、callableなオブジェクトならJsObjectとして返す。ここで返ってきたものがitemsから引き続き到達可能かどうかは、GetMethodの返り値だけでは保証されない。

items --getter--> 新しいiterator関数
                   |
                   +-- Boaの一時変数だけが保持

出力constructorの実行中にGC
                   |
                   +-- その後で関数を呼ぼうとする

正確には、GetMethodがundefinedを返した場合はarray-likeとして処理する別の経路に入る。今回の穴は、呼び出し可能なiteratorメソッドが見つかったiterable側の経路にある。getterで新規作成した関数という条件も、普段の配列をArray.from()に渡すだけでは露出しにくかった理由の一つだ。

Boaの修正箇所でも、まずitems.get_method(JsSymbol::iterator(), context)?でメソッドを得る。Some(using_iterator)ならiterable側へ進み、出力先を生成してからitems.get_iterator_from_method(&using_iterator, context)?を呼ぶ。後者の実装は、渡された関数をmethod.call(self, &[], context)で実際に呼ぶ。問題になるのは、メソッド取得と呼び出しの間にある出力生成だ。途中でユーザー定義のconstructorへ制御を渡している。

Rustの変数とGCのrootは別

BoaはGetMethodの戻り値をRustのusing_iteratorというローカル変数に入れていた。しかし、Rustの変数がスコープ内にあることと、JavaScriptのGCへ「このオブジェクトは生きている」と登録することは同じではない。特にこの経路では、getterの戻り値がほかのJavaScriptオブジェクトから参照されているとは限らなかった。

Boaの実装には、ヒープ中の参照を表すJsObjectと、ヒープの外から明示的に保持するrootの区別がある。root()は外部rootとして登録されたハンドルを作る。ここで必要だったのは、メソッド取得後からiterator取得までの間、取得した関数をGCが到達可能な値として扱うことだった。

この区別は「Rustが所有権を管理しているのだから、変数が残っていれば大丈夫」という直感とずれる。JsObjectの定義を見ると、内部にはGcEdgeがある。GcEdgeの説明は、GCに追跡されるヒープの中に置く参照であり、それ自体を外部root集合へ登録しないというもの。JsObjectをclone()しても同じ対象への参照が増えるだけで、nativeコードのローカル変数をrootとして登録したことにはならない。

対してJsObject::root()はRootedJsObjectを返す。Rootedの実装では生成時にregister_rootし、ハンドルが破棄されるとunregister_rootする。root登録はGC対象の割り当てに対して数えられ、0個から1個になったときにroot registryへ載る。GCのmark処理はそこに登録された対象からたどり始める。単にRust側にポインタがある状態と、GCの探索開始点に入っている状態は違う。

修正前: getterが返した関数 -> JsObject / GcEdge
                           -> nativeローカル変数にあるが、外部root未登録

修正後: getterが返した関数 -> RootedJsObject
                           -> root registryからGCが到達可能

もちろんBoaのrootはこのregistryだけではない。VMの値スタックもRootProviderとして登録され、GCはそこに残っている値も調べる。この仕組みは後で、最初のJavaScript getter版テストが問題を再現しなかった理由につながる。rootが「ないはず」の関数にも、たまたま別の実行フレームから到達できる場合がある。

重要なのは、関数を永久に残すことではない。GetMethodの結果を受け取ってから、constructorでGCが起こり得る区間を越え、GetIteratorFromMethodがその関数を呼ぶまでが必要な生存期間だ。そこを明示的なrootで覆えばよく、iteratorを取得した後まで入力オブジェクトのプロパティを書き換えて関数を保持する必要はない。

vtableで落ちたことから、どこまで言えるか

LLDBの停止位置と、後述する修正前後のテストを合わせると、取得した関数の寿命が保証されないまま呼び出しに進んでいた、という説明が最もよく合う。ただし、停止した瞬間のログだけで「このGCサイクルでこのアドレスが回収され、別の割り当てに再利用された」とまで追跡できたわけではない。ここは診断の強さを分けておきたい。

実際に確認できたのは、Array.from()から渡された関数を呼ぶ経路でvtable参照がSIGSEGVになったこと、constructorでGCを強制する小さなテストが修正前に失敗したこと、そしてその関数をrootすると小さなテストと元のAcid3診断がともに成功したことだ。この組み合わせは参照保持漏れを強く支持するが、macOSのデバッガログ単体でメモリ再利用まで証明したわけではない。

また、JIT有効時だけ観測したCIの症状から、JITのコード生成を直すべきだと即断するのも違う。JSエンジンでは、同じJavaScriptを動かしていても、interpreterかJITか、OSやアーキテクチャが何かでレジスタの使い方やオブジェクトの寿命の見え方が変わる。この差がGCのバグを隠したり露出させたりすることがある。後でLinux ARM64の単体テストでも失敗したため、少なくとも原因をmacOSのJITに限定する説明は成り立たない。

強制GCを入れても最初のテストは通ってしまった

落ちたCIだけを繰り返しても、修正が本当に原因へ効いたか判断しにくい。そこで、getterが新しいiterator関数を返し、出力constructorの中で強制GCを実行する小さな回帰テストを作った。

狙いはAcid3全体を縮めることではなく、先ほどの参照グラフを最小限で再現すること。入力オブジェクトとgetterそのものは生かしたまま、getterの返した関数だけがどこにも保存されない状態を作る。そして、GetMethodの直後ではなく、出力constructorの中でGCを実行する。もしGCをgetterより前に実行しても、対象の一時的な関数はまだ存在しない。iteratorを呼んだ後で実行しても、今回の問題の区間を通らない。

最初はgetterをJavaScriptで書いたが、修正前でも再現しなかった。getterの実行を終えたJavaScriptフレームのレジスタに戻り値が残り、意図しない別の参照として関数を生かすことがある。GCのテストでは、「本来ないはずのrootがたまたま残る」という偽陰性が厄介だ。

これは「JavaScriptから書いたgetterならバグが起きない」という意味ではない。テストの特定の実行経路では、使い終わったはずのフレームに残る値が偶然保護してくれた、というだけだ。先ほど見たBoaのGCはVMの値スタックもrootとしてたどる。getterをJavaScriptで実行すると、その戻り値がVM側に残るかどうかが再現性に効いてしまう。強制GCを入れたのに落ちなかったという事実だけでは、参照保持が正しいとは言えなかった。

最終的な回帰テストでは、Rust側のnative getterから関数を生成して返す。出力constructorではテスト用のforceCollect()を呼び、続けて取得済みの関数が使われるかを調べる。

実装ではsourceをrootし、Symbol.iteratorにはnative getterを設定する。getterの方も必要な期間はrootされている。しかしgetterが返す新しいiterator関数はsourceのプロパティに格納しない。ここでソースやgetter自体が回収されてしまうと、見たい問題と別の失敗になるため、この区別が必要だった。テスト中のJavaScript側は次のような形だ。forceCollect()はテストが登録した関数で、一般のWebページで使えるAPIではない。

let calls = 0;
function Destination() {
  forceCollect();
  this.marker = "destination";
}

const result = Array.from.call(Destination, source);
result instanceof Destination &&
  result.marker === "destination" &&
  result.length === 1 &&
  result[0] === 37 &&
  calls === 1;

getterが返す関数は、呼ばれたらcallsを増やし、[37][Symbol.iterator]()を返す。結果が37であることだけでなく、出力オブジェクトの型、constructorが付けた印、配列の長さ、iterator関数の呼び出し回数まで確認する。ここまで見ないと、クラッシュを回避したつもりで別の経路を通っただけ、という可能性が残る。

native getterが新しいiterator関数を返す
  -> Array.fromが出力constructorを実行
  -> constructorがGCを強制
  -> 保存していたiterator関数を呼ぶ
  -> 結果の型、値、長さ、呼び出し回数を検査

これなら修正前のLinux ARM64でも異常終了し、停止経路はAcid3で見たArray.from()からのiterator呼び出しと一致した。macOSの偶発的なCI失敗として片付けず、独立した小さなテストで同じ寿命の問題を確認できた。

再現性の確認は、最初から修正入りで通るテストを書くより重要だった。修正前のソースに戻して失敗し、修正後に同じテストが通る。これで、テストが本当に修正対象の穴を踏んでいることを確かめられる。Linux ARM64でも失敗したため、macOSのrunnerやLLDBがないと検証できない回帰にもならなかった。

修正はconstructorの前でrootする

修正コミットでArray.from()に加えた本体は、次の1行。

let using_iterator = using_iterator.root();

配置したのはGetMethodで関数を取得した後、出力constructorやArrayCreateへ進む前だ。rootはその後のget_iterator_from_methodでも使う。呼び出し順序を入れ替えたり、getterの戻り値を入力オブジェクトへ勝手に保存したりはしていない。どちらもJavaScriptから観測できる動作を変えてしまうからだ。GCを止めるのでもなく、仕様が要求する順序のまま、必要な期間だけ関数の寿命を保証する修正になった。

letで同じ名前を使っているが、単なる変数名の付け替えではない。右辺のusing_iteratorはJsObjectで、左辺はroot()の返したroot付きのハンドルになる。後のget_iterator_from_methodはそのrootを保持したまま呼ぶ。呼び出しが終わって関数のスコープを抜ければ、RootedのDropがroot登録を解除する。途中のconstructorが例外を投げ、Rustの?で早く戻る場合にも、rootをその場に置き去りにはしない。

コード上は、出力オブジェクトaも生成後にa.clone().root()で保持されている。しかしそれは出力オブジェクトの寿命を守るrootであり、getterが返したiterator関数の寿命を守るものではない。出力先が後からその関数を参照する保証はない。ここを同じ「Array.from()の途中で使うオブジェクト」とまとめてしまうと、保持すべき対象を取り違える。

同様に、using_iterator.clone()を増やすだけでも外部root登録は増えないし、sourceをrootしてもgetterの返り値はsourceに保存されていない。テストでは実際にsourceをrootしている。それでも修正前に失敗したことが、この参照関係をよく示している。

rootを置く場所も重要だ。constructorの後にrootしても、そのconstructor内でGCが起きるなら間に合わない。一方、GetMethodがundefinedを返したarray-like経路まで無条件に関数をrootする必要はない。Some(using_iterator)と分かった直後にrootする今回の位置は、必要な経路と生存期間に合っている。

三段階で修正を確かめる

修正後、先ほどの回帰テストは成功した。Boaエンジンのbaseline JIT有効時の単体テスト1,050件、Omoikane全体の2,694件も成功している。さらに同じ3行の修正を問題のrevisionに当てたmacOS診断CIでは、通常4回とLLDB付き4回のすべてでAcid3の両経路が100/100になった。テストの時間制限やfixtureは変えていない。

ここでは検証の役割を分けている。まず、回帰テストの修正前失敗・修正後成功は、狙った参照保持の穴を踏んでいるかを見る。次に、問題が起きたPR revisionへ参照保持の3行だけを適用したmacOS診断は、ほかの変更を一緒に入れなくても元の症状が消えるかを見る。最後に最終PR全体のCIで、周辺の機能を含めて退行がないかを見る。成功したテストの数だけを足し合わせるより、この区別の方が大事だ。

最終PRのCIでも、Linux x86_64、Linux ARM64、macOS ARM64のinterpreterとbaseline JIT、計6構成のBrowserテストが成功した。Test262比較では各環境50,595ケースを実行し、この修正による退行やpanic、3環境間の結果差はなかった。既知の未対応ケースは残っており、全件準拠という意味ではない。

フォーム関連機能のPRだったため、固定WPTの確認も継続している。147件中146件がPASS、1件は既知の未対応で、新しい退行はなかった。こちらはGCの原因を直接証明するテストではないが、Boa側の修正を取り込んでもブラウザの既存動作を壊していないかを見る材料になる。

もちろん、これらの結果でBoaのすべてのGetMethod呼び出しが安全だと証明できたわけではない。確認したのは、このArray.from()の区間を狙った回帰と、PRで維持しているテスト群だ。新しいrootの追加による細かな性能差も測っていないので、「安全になって性能も変わらない」という主張にはしない。

同じ種類の問題をどう探すか

今回の発見場所はmacOSのbaseline JITとAcid3だったが、原因はもっと手前にあるBoaの一時参照の寿命だった。Linux ARM64でも切り出した回帰テストを失敗させられたため、macOSやJITだけの問題とは扱わない。実行構成ごとの成功・失敗は、再現条件を探す手掛かりにはなるが、原因となるコードの境界をそのまま示すわけではなかった。

似た問題を見るなら、まず「いつ値を取得し、いつ最後に使うか」を並べる必要がある。今回の値はGetMethodが返したiterator関数で、最後の使用はGetIteratorFromMethodからの呼び出しだった。次に、その間の処理で何が実行されるかを調べる。Array.from()の場合は出力先のconstructorが入り、ここで任意のJavaScriptが動く。GCを直接呼べるテスト環境でなくても、JavaScriptを実行できる区間は割り当てや回収の可能性を検討すべき場所になる。

そのうえで、その値への参照経路を描く。元のオブジェクトが関数をプロパティとして持っているなら、そこから到達できる場合もある。しかしgetterが作った関数なら、getterを持つオブジェクトがその戻り値まで所有しているとは限らない。「入力オブジェクトをrootしたから大丈夫」という判断が、この区別を見落とす。RustのJsObjectがスコープ内にあっても、それだけではGCのroot集合に入らないことも確認する必要があった。

テストでは、保護したい値以外の条件を固定する。今回なら入力オブジェクトとgetterを生かし、返された関数だけを一時参照にした。出力先のconstructorで強制GCを起こし、その後の呼び出し結果まで確認する。単にプロセスが落ちなかったことだけではなく、iteratorが一度呼ばれ、正しい出力先に正しい値が入ったことを検査する。修正前に失敗するのを確かめなければ、最初のJavaScript getter版のように、偶然残ったVMスタック上の値に保護されているかもしれない。

この順で見ていくと、修正の対象も絞れる。GCを止める、iteratorの呼び出しをconstructorより前に動かす、入力オブジェクトへ関数を書き戻す、といった方法は不要か、仕様上の動作を変えてしまう。必要だったのは、取得した関数を危険な区間だけrootすることだった。例外で処理が中断されてもRootedの破棄で登録が外れるため、保持期間はRustの制御フローにも沿う。

ただし、この手順でArray.from()の今回の穴を塞げたことと、エンジン全体の参照保持を監査できたことは別だ。ほかの組み込み関数にも「取得したJavaScript値をnative側で持ち、途中でユーザーコードを呼んでから再利用する」場所はあり得るが、この記事ではそれらを調べ切っていない。また、vtableでのSIGSEGVを観測したログから、どのGCサイクルでメモリが再利用されたかまで特定したわけでもない。ここで確認できたのは、参照保持の欠落を再現するテストが修正前に失敗し、対象の関数をrootするとテストとAcid3の症状が解消した、という範囲だ。

今回のバグは以前のPromiseの参照保持漏れやshared shapeの弱参照キャッシュ問題とは別の箇所だった。似たクラッシュに見えても、どの値を、どの処理の間、生かしておく必要があるかを一つずつ追うしかない。1行の修正より、そこを特定できる回帰テストを作れたことの方が大きい。