rescue節以外で$!を使ってはいけない

Posted by Shugo Maeda on December 24, 2023 · 1 min read

NaClの前田です。
Ruby Advent Calendar 2023の24日目の記事です。昨日は@ydahさんでした。

今回は、弊社副社長の後藤(@gotoyuzo)から報告されたnet-ftpのバグを紹介します。

バグの内容

報告されたバグの再現コードは以下のようなものです。

require "net/ftp"

begin
  raise "FOO!"
rescue
  ftp = Net::FTP.new("ftp.example.com")  # specify an anonymous FTP server
  ftp.login
  p ftp.list
end

上記のコードを実行すると、サーバが正常に動作している場合でもclosed stream (IOError)という例外が発生します。

バグの原因

原因はnet-ftpの以下のコードでした。

begin
  conn = open_socket(host, port)
  ...
ensure
  conn.close if conn && $!
end

これは6e8535fで導入されたコードで、FTPのデータコネクションの確立中に例外が発生したら、ソケットをクローズしてコネクションリークを防ぐためのコードです。

一見問題ないように見えますが、$!のスコープはスレッドローカルなため、データコネクションの確立が例外なく完了した場合も、呼び出し元で例外が発生していた際にはconn && $!という条件が真になってソケットがクローズされてしまいます。

$!をrescue節以外で使用するとこのような問題があるので気をつけましょう(そもそもワンライナーなど以外では使用しない方がいいと思いますが)。

修正案1

後藤から提案された修正は以下のようなものでした。

begin
  conn = open_socket(host, port)
  ...
rescue
  conn&.close
  raise
end

このコードではこのbegin式で例外が発生した場合だけソケットがクローズされるようになるため、報告された問題は解消します。

修正案2

ところが修正案1に対してJeremy EvansさんからStandardError以外の例外が発生した場合にコネクションリークが起きてしまうという指摘がありました。

例外クラスを指定しないrescueは、StandardErrorおよびそのサブクラスのみを捕捉するためです。

この問題に対処するためにさらに以下のような修正を行いました。

begin
  conn = open_socket(host, port)
  ...
rescue Exception
  conn&.close
  raise
end

最終的な修正内容

これで解決かと思ったのですが、さらにSamuel WilliamsさんからThread#killなどで実行が中断された場合にコネクションリークが起きてしまうという指摘がありました。

そこで最終的には以下のような修正を行いました。

succeeded = false
begin
  conn = open_socket(host, port)
  ...
  succeeded = true
ensure
  conn&.close if !succeeded
end

このコードでも厳密にはbegin式の終了から呼び出し元にリターンするまでの間にThread.killなどで実行が中断されるとコネクションリークが発生しますが、まれなケースなので対処しなくてもいいかなと思っています(と思ってたけど、今コード見てたら@private_data_connectionが真の時はbeginの終了後にstart_tls_session(conn)しているのでやっぱり対処した方がいいかも……)。

まとめ

← Previous Post Next Post →