🐛

Deno のそこそこ難しいバグを修正(?)した

2022/10/12に公開

JavaScript

2021年の2月、Denoに以下のissueが投稿されました。

このissueによると、すでに作成済みのTCPコネクションをベースにしてTLS接続を確立すると、Denoがパニックして落ちてしまうということです。

パニック時のログは次のような感じです。（抜粋。読みやすいように改行を入れています）

thread 'main' panicked at 'Only a single use of this resource should happen:
FullDuplexResource {
  rd: AsyncRefCell<tokio::net::tcp::split_owned::OwnedReadHalf>,
  wr: AsyncRefCell<tokio::net::tcp::split_owned::OwnedWriteHalf>,
  cancel_handle: CancelHandle { 
    node: Node { 
      inner: UnsafeCell,
      _pin: PhantomPinned,
    },
  },
}', runtime/ops/tls.rs:126:6

このissueはstale bot（しばらくの期間アクティビティがないissueを自動でクローズするbot）によってクローズされてしまっていますが、その後も以下のように同様の事象が何件も報告されています。

これは対応したほうが良いということで、調査を開始することにしました。

まずissueで報告された事象が現在のDenoでも再現するかどうかを確認します。そのために、事象を常に発生させることができ、かつコード量も可能な限り小さいような最小再現コードを探ります。

#13345 のコメントによると、issue投稿者が比較的小さい再現コードを見つけてくれたようです。次のようなコードです。（一部フォーマットを整えています）

Deno.connect({ transport: "tcp", port: 5432, hostname: "localhost" }).then(
  async (conn) => {
    await conn.write(new Uint8Array([0, 0, 0, 8, 4, 210, 22, 47]));
    const b = new Uint8Array(128);

    while (await conn.read(b)) {
      if (b[0] === 83) {
        connect(conn);
        // adding break here fixes the issue
      }
    }
  },
);

async function connect(conn) {
  // This is the reason it panicks, and if I remove it
  // I get a more descriptive BadResource: Bad resource ID error
  await 0;

  Deno.startTls(conn, { hostname: "localhost" });
}

やっている内容は次のような感じかなと思います。

Deno.connect でTCPコネクションを張る
TCPでデータを送信
TCPでデータを受信
受信したデータの最初のバイトが 83 だったら、connect() 関数を呼ぶ。この関数は特に意味のない await 0 と、TLS接続を確立するための Deno.startTls() から成る

自分の手元でDenoの最新バージョン（執筆時点で1.26.1）で試したところ、確かに上記のコードで確実にパニックすることが分かりました。

再現コードが得られたところで、実際にDenoのコードを追いながら原因に迫っていくことにします。まずパニック時のログを見ると、パニックが発生した場所のファイル名と行数が書いてあるので、その周辺から眺めることにします。この記事の冒頭に貼ったログはDenoの古いバージョンのものなので、パニック発生箇所は最新バージョンのものとは異なることに注意です。

↑がパニック発生箇所（とその周辺）ですが、パニックが発生している理由自体は明白で、Rustの標準ライブラリで提供されている std::rc::Rc::try_unwrap が失敗しているにも関わらず expect() を呼び出しているからです。

Rustに馴染みがない方にざっくりと説明をすると、基本的にはRustの値は「所有者」が唯一存在することになっているのですが、Rc を使うと所有者が複数存在することができるようになります。内部的には、何人の所有者がいるのかをカウンタで管理することで、適切にリソース管理を行ってくれます。Rcは "Reference Counted" （参照カウント）の略です。そして、Rc::try_unwrap は、Rcで管理されている値をRcの管理から外すことを試みるメソッドです。ただし、先に書いたように、一般的にRustの値は所有者が唯一存在することになっているため、Rcの管理から外すことができるのは、参照カウントが1である場合のみです。正確には強参照とか弱参照とかもう少し細かい概念があるのですが、ここではとりあえず大雑把な説明にとどめておきます。

そして、上記のコードでは、「このコードパスを通るときには、必ず参照カウントが1であるはずなので、そうじゃなかったら何かがおかしい」という「前提」のもと、Rc::try_unwrap の結果に対して expect() が使われています。パニックするということは、この「前提」が間違っているということです。

どういう場合にこの「前提」が崩れるのでしょうか。それを探るために、まずDenoの内部構造について簡単に触れます。Denoは内部でリソーステーブルというものを管理しています。「リソース」はOSの「ファイル」に対応するような概念で、例えばファイルをオープンしたらリソーステーブルにリソースとして追加される、ソケットを開いたらリソーステーブルに追加される、などです。OSがファイルディスクリプタで管理しているようなものを、Denoも独自で管理しているというような感じです。いま調査しているケースに関しては、TCPソケットを作っているので、それがリソーステーブルに追加されることになります。このリソーステーブルで管理されているリソースが、Rcに包まれていて、参照カウントによって管理されているということです。そして、パニックの発生箇所においては、「このTCPソケットというリソースを今現在参照しているのは自分だけであるに違いないので、Rcの管理から外して自分が所有者ということにさせてください」という処理をしていることになります。

つまり、パニックの原因は、他の何者かがTCPソケットを使用中であるためということになります。では、誰がどのように使っているのか？これを調べるために再現コードをにらめっこします。しかし、ここまでの考察をベースに考えてみると、上記で掲載した再現コードは、まだ少し無駄があるようにも感じられます。"Deno.startTls() を実行するタイミングで、他の誰かがTCPソケットを参照しているような状況" をできる限り少ないコードで実現すれば良さそうです。以下のようなコードを作ってみました。

// TCPコネクションを張る
const conn = await Deno.connect({
  transport: 'tcp',
  port: 25432,
  hostname: 'localhost',
});

// 相手から何らかのデータを受け取る
const buf = new Uint8Array(128);
const promise1 = Deno.read(conn.rid, buf); // 注意: await していない

// TCPコネクションをベースにTLS接続を行う
await Deno.startTls(conn, { hostname: 'localhost' });

このコードでもパニックが再現することが確認できました。Deno.startTls() を実行するタイミングで、Deno.read() がTCPコネクションを使用中であるのがポイントです。これで無駄なく "Deno.startTls() を実行するタイミングで、他の誰かがTCPソケットを参照しているような状況" を作り出すことができました。

さて、「前提」が崩れるようなシチュエーションが把握できたので、どうやって直していくかを考えていきます。いくらかの検討を行った結果、取りうるアプローチとして以下の2つを思いつきました。

そもそも「前提」が間違っているのだから、Rc::try_unwrap の結果に対して expect() を呼ぶのをやめて、適切にエラーを返すようにする。つまり、他の何者かがTCPソケットを参照している間に Deno.startTls() が実行されたら、JavaScript側にエラーで返してあげるようにする。
Rc::try_unwrap を実行する必要が本当にあるのかを考える。TCPソケットをリソーステーブル（Rcの管理下）から外さずに済む方法は無いのか？

1は素直な解決策です。現在は問答無用でパニックしてしまっているところを、適切にエラーを返してあげるようにします。少なくともパニックするよりもマシだろうとは思いますが、結局 Deno.startTls() が失敗することには変わりありません。

2はもう少し踏み込んだアプローチです。TCPソケットをRcの管理から外すということ自体のモチベーションを追ってみます。コードを読み込んでみると、Rcの管理から外すのは、既存のTCPをベースにTLS接続を行う際に、TCPソケットに対する所有権が必要であるというRustのコード上の制約があるから、というように見えました。Deno.startTls() では、渡されたTCPソケットを「消費」して新しいTLS接続を作成します。この過程で元のTCPソケットはリソーステーブルから削除され、新しいTLS接続を示すリソースがテーブルに登録されます。ユーザーの視点から言うと、Deno.startTls() を呼んだあと、元のTCPコネクションは利用不可になるということです。これはこれで別のissueが立てられていることを発見しました。

（こちらは Deno.startTls() ではなく Deno.serveHttp() についてですが、背景は全く同一です）

"既存のTCPソケットを「消費」しない形で、新しいリソースとしてTLS接続を作成してリソーステーブルに登録したい"
という要求を実現できれば良さそうですが、これを叶える手段はないかといろいろ調べていたところ、Rustの標準ライブラリのTCP接続を表す構造体に std::net::TcpStream::try_clone というメソッドが用意されていることに気づきました。これを使うと、元のTcpStreamの所有権を消費せず、新しいTcpStreamを作成することができます。内部的には、fnctl(2) に F_DUPFD_CLOEXEC フラグとともに元のTCPソケットのファイルディスクリプタを渡すことで、ソケットを複製しているようです。

残念なことに、Denoが利用している非同期ライブラリのtokioが提供している tokio::net::TcpStream には、try_clone のメソッドは存在していません。issue は立っていますが、特に進展はなさそう。何らかのやり方はあるかもしれませんが、OSごとの細かい挙動の違いや、tokioランタイムとの整合性のことなどを考えると、考えなければならないことが飛躍的に増えてしまいそうです。このあたりまで検討して、アプローチ2はいったん諦めることにしました。

ということで、アプローチ1を採用したPRを作成しました。

パニックではなくエラーを返すようにした + そのテストケースを書いたのに加えて、ユーザーに見えるドキュメント部分にも、どのような場合にエラーが返ってくるのかについての説明を追加しました。この記事のタイトルに "(?)" がついているのは、完全なる解決というよりはその場しのぎ的な解決になったためです。

Inspired by

この記事は以下のkt3kさんの記事にインスパイアされています。

Inspired by

Discussion