Bug #8730
closed"rescue Exception" rescues Timeout::ExitException
Description
=begin
Timeout.timeout ブロック内で rescue Exception によって例外を捕捉している箇所があると、
タイムアウト処理が内部的に利用している Timeout::ExitException クラスの無名派生クラスを補足してしまい、
正しく Timeout::Error が発生しない。
例)
timeout 1 do
begin
sleep 3
rescue Exception => e
puts e.class.superclass #=> "Timeout::ExitException"
end
end
=end
Updated by nobu (Nobuyoshi Nakada) over 11 years ago
- Status changed from Open to Rejected
Exception全体をrescueして握り潰しているのがバグでしょう。
Updated by kosaki (Motohiro KOSAKI) about 11 years ago
ちょっと待った。
現在の挙動は様々な書籍などでも解説されており広く共有されている挙動なので
NEWSも書かずに変えてしまうのは適切じゃないように思います。
そもそもどこで議論した結果?
Updated by nobu (Nobuyoshi Nakada) about 11 years ago
具体的にはどのように解説されてどう利用されているんでしょうか。
Updated by kosaki (Motohiro KOSAKI) about 11 years ago
具体的にはどのように解説されてどう利用されているんでしょうか。
残念ながら紙の書籍は手元にありませんが、
一番多いのは http://kwfsws.g.hatena.ne.jp/kiwofusi/20111231/1325314356 みたいに
rescue Exception => e とすると、意図せずタイムアウトも拾ってしまうので注意しましょうという記述。
これは "ruby timeout 例外" で検索すると似たような注意は何個も出てきます。
次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう(書いている)という記述 http://d.hatena.ne.jp/dreammind/20090217/1234813224
2番めは動かなくなりますよね。
それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。
Updated by nobu (Nobuyoshi Nakada) about 11 years ago
kosaki (Motohiro KOSAKI) wrote:
具体的にはどのように解説されてどう利用されているんでしょうか。
残念ながら紙の書籍は手元にありませんが、
一番多いのは http://kwfsws.g.hatena.ne.jp/kiwofusi/20111231/1325314356 みたいに
rescue Exception => e とすると、意図せずタイムアウトも拾ってしまうので注意しましょうという記述。
これは "ruby timeout 例外" で検索すると似たような注意は何個も出てきます。
なるほど。
次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう(書いている)という記述 http://d.hatena.ne.jp/dreammind/20090217/12348132242番めは動かなくなりますよね。
Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。
それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。
意図せずにrescueされてしまう問題点は以前から知られていた問題なので、今回のチケットをきっかけにちょっと試してみたわけです。
非互換性として思いついたのは、timeoutされる処理の中でExceptionをrescueしてtimeoutを起こさないようにしていた場合ですが、あるとしたらむしろ意図しないバグだろうと考えました。
前者はまさにその一例ですね。
Updated by takiuchi (Genki Takiuchi) about 11 years ago
すでに修正されているようなので蛇足気味ですが、
githubなどで公開されているソースコードを調べますと、
rescue Exception
を使っているライブラリコードは広く散見され、
書籍などで紹介されている workaround はほとんど浸透していないようです。
この問題の厄介なところは、自分自身が書いたコードだけでなく、利用するライブラリの
コード中にも rescue Exception
をしている箇所があると、大外に掛けた timeout が無効化
されてしまう可能性があり、それを予見するのが難しいという点です。
典型的な例としては、以下の様なネットワーク処理を行うライブラリの挙動に
タイムアウトを設定したいというケースがあると思います。
timeout 10 do
CoolHttpClient.get "http://foo.bar.com"
end
この場合、CoolHttpClient の内部実装のどこかで rescue Exception している箇所が1つでも
あれば、タイミングによっては timeout がもみ消されてしまいます。
他方で、ライブラリ作者としては大外から timeout ブロックで囲われるケースを想定して、
rescue Exception
と書いてはならない事になりますが、ここまで来るとやはり
バグであると考えるべきだと思います。
Updated by mame (Yusuke Endoh) about 11 years ago
timeout の変更自体について特に意見はないのですが。
rescue Exception
は timeout に限定した問題ではなく、
exit ができないとか、Ctrl-C が効かないとか、トラブルになりがちです。
大した理由もなくそういうことをしているライブラリには
ちゃんと対応してもらうべきだと思います。
また、throw にすれば万事解決というわけではないです。
ensure を使って例外を差し替えることで同じ現象を引き起こせます。
timeout 1 do
begin
begin
sleep 3
ensure
raise
end
rescue
end
end
--
Yusuke Endoh mame@tsg.ne.jp
Updated by kosaki (Motohiro KOSAKI) about 11 years ago
2013/8/28 nobu (Nobuyoshi Nakada) nobu@ruby-lang.org:
Issue #8730 has been updated by nobu (Nobuyoshi Nakada).
kosaki (Motohiro KOSAKI) wrote:
具体的にはどのように解説されてどう利用されているんでしょうか。
残念ながら紙の書籍は手元にありませんが、
一番多いのは http://kwfsws.g.hatena.ne.jp/kiwofusi/20111231/1325314356 みたいに
rescue Exception => e とすると、意図せずタイムアウトも拾ってしまうので注意しましょうという記述。
これは "ruby timeout 例外" で検索すると似たような注意は何個も出てきます。なるほど。
次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう(書いている)という記述 http://d.hatena.ne.jp/dreammind/20090217/12348132242番めは動かなくなりますよね。
Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。
いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。
それを最初のメールで補足するべきでしたね。
それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、(ログを吐くなどした後)もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。
なので、副作用のほうが気になるわけです。
それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。
意図せずにrescueされてしまう問題点は以前から知られていた問題なので、今回のチケットをきっかけにちょっと試してみたわけです。
非互換性として思いついたのは、timeoutされる処理の中でExceptionをrescueしてtimeoutを起こさないようにしていた場合ですが、あるとしたらむしろ意図しないバグだろうと考えました。
前者はまさにその一例ですね。
前者がバグであることについては完全に同意します
Updated by nobu (Nobuyoshi Nakada) about 11 years ago
(13/09/05 0:25), KOSAKI Motohiro wrote:
次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう(書いている)という記述 http://d.hatena.ne.jp/dreammind/20090217/12348132242番めは動かなくなりますよね。
Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。
いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。
そのblogにあるコードでは、timeoutが発生するのはnet/httpの中、捕まえるのは外なので、今回の変更による影響はないということです。
StandardErrorで捕まえられるか否かはこれとはまた別件です。
それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、(ログを吐くなどした後)もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。なので、副作用のほうが気になるわけです。
Exceptionで丸ごと握りつぶしてしまうのがまずいことはまったく同意ですが、完全ではないにせよそれを避けられるほうがいいのではないでしょうか。
Updated by kosaki (Motohiro KOSAKI) about 11 years ago
(9/5/13 3:31 AM), Nobuyoshi Nakada wrote:
(13/09/05 0:25), KOSAKI Motohiro wrote:
次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう(書いている)という記述 http://d.hatena.ne.jp/dreammind/20090217/12348132242番めは動かなくなりますよね。
Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。
いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。
そのblogにあるコードでは、timeoutが発生するのはnet/httpの中、捕まえるのは外なので、今回の変更による影響はないということです。
StandardErrorで捕まえられるか否かはこれとはまた別件です。
なるほど
それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、(ログを吐くなどした後)もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。なので、副作用のほうが気になるわけです。
Exceptionで丸ごと握りつぶしてしまうのがまずいことはまったく同意ですが、完全ではないにせよそれを避けられるほうがいいのではないでしょうか。
趣旨はだいたい分かりました。
では NEWSに追記だけしてもらえますか。これで壊れるコードがないかは依然として不明ですが、リスクは低いと思うので
アプリケーション側を直してもらうというのでいいような気がしてきました
Updated by takiuchi (Genki Takiuchi) almost 11 years ago
そもそも rescue Exceptionを使うのはまずいという意見が出ていますが、
例えば以下ようにフィルタ的に全てのexceptionを補足してそのままraiseするような
利用をしたい場合があり、実際に使われています。
begin
do_something
rescue Exception => e
log_exception e
raise e
end
libraryコードの中でこのようなコードが一箇所でもあると、大外からtimeoutを掛ける事が
できないので、やはり従前の挙動は問題が多いと思います。
Updated by nobu (Nobuyoshi Nakada) almost 11 years ago
そういう場合は
begin
do_something
ensure
log_exception $!
end
のほうがいいんじゃないかと。
Updated by nobu (Nobuyoshi Nakada) almost 11 years ago
すいません、常にログしてはだめですね。
log_exception $! if $!
Updated by byroot (Jean Boussier) over 1 year ago
- Related to Misc #19740: Block taking methods can't differentiate between a non-local return and a throw added