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節以外で使用するとこのような問題があるので気をつけましょう(そもそもワンライナーなど以外では使用しない方がいいと思いますが)。
後藤から提案された修正は以下のようなものでした。
begin
conn = open_socket(host, port)
...
rescue
conn&.close
raise
end
このコードではこのbegin式で例外が発生した場合だけソケットがクローズされるようになるため、報告された問題は解消します。
ところが修正案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)
しているのでやっぱり対処した方がいいかも……)。
$!
を使わないようにしましょう。conn&.close
の部分)が入ります。