Bug #8730: "rescue Exception" rescues Timeout::ExitException

• Status: Rejected
• Assignee: -
• Target version: 2.1.0
• ruby -v: 2.0.0
• Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN

[takiuchi (Genki Takiuchi)]

=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

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Rejected

Exception全体をrescueして握り潰しているのがバグでしょう。

[kosaki (Motohiro KOSAKI)]

ちょっと待った。
現在の挙動は様々な書籍などでも解説されており広く共有されている挙動なので
NEWSも書かずに変えてしまうのは適切じゃないように思います。

そもそもどこで議論した結果？

[nobu (Nobuyoshi Nakada)]

具体的にはどのように解説されてどう利用されているんでしょうか。

[kosaki (Motohiro KOSAKI)]

具体的にはどのように解説されてどう利用されているんでしょうか。

残念ながら紙の書籍は手元にありませんが、

一番多いのは 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

２番めは動かなくなりますよね。

それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。

[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/1234813224

２番めは動かなくなりますよね。

Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。

それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。

意図せずにrescueされてしまう問題点は以前から知られていた問題なので、今回のチケットをきっかけにちょっと試してみたわけです。
非互換性として思いついたのは、timeoutされる処理の中でExceptionをrescueしてtimeoutを起こさないようにしていた場合ですが、あるとしたらむしろ意図しないバグだろうと考えました。
前者はまさにその一例ですね。

[takiuchi (Genki Takiuchi)]

すでに修正されているようなので蛇足気味ですが、

githubなどで公開されているソースコードを調べますと、
rescue Exception を使っているライブラリコードは広く散見され、
書籍などで紹介されている workaround はほとんど浸透していないようです。

この問題の厄介なところは、自分自身が書いたコードだけでなく、利用するライブラリの
コード中にも rescue Exception をしている箇所があると、大外に掛けた timeout が無効化
されてしまう可能性があり、それを予見するのが難しいという点です。

典型的な例としては、以下の様なネットワーク処理を行うライブラリの挙動に
タイムアウトを設定したいというケースがあると思います。

timeout 10 do
  CoolHttpClient.get "http://foo.bar.com"
end

この場合、CoolHttpClient の内部実装のどこかで rescue Exception している箇所が１つでも
あれば、タイミングによっては timeout がもみ消されてしまいます。
他方で、ライブラリ作者としては大外から timeout ブロックで囲われるケースを想定して、
rescue Exception と書いてはならない事になりますが、ここまで来るとやはり
バグであると考えるべきだと思います。

[mame (Yusuke Endoh)]

timeout の変更自体について特に意見はないのですが。

rescue Exception は timeout に限定した問題ではなく、
exit ができないとか、Ctrl-C が効かないとか、トラブルになりがちです。
大した理由もなくそういうことをしているライブラリには
ちゃんと対応してもらうべきだと思います。

また、throw にすれば万事解決というわけではないです。
ensure を使って例外を差し替えることで同じ現象を引き起こせます。

  timeout 1 do
    begin
      begin
        sleep 3
      ensure
        raise
      end
    rescue
    end
  end

--
Yusuke Endoh [e-mail]

[kosaki (Motohiro KOSAKI)]

2013/8/28 nobu (Nobuyoshi Nakada) [e-mail]:

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/1234813224

２番めは動かなくなりますよね。

Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。

いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。

それを最初のメールで補足するべきでしたね。

それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、（ログを吐くなどした後）もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。

なので、副作用のほうが気になるわけです。

それでどこで議論して、Pros/Consをどう判断した結果なのかとお聞きしました。

意図せずにrescueされてしまう問題点は以前から知られていた問題なので、今回のチケットをきっかけにちょっと試してみたわけです。
非互換性として思いついたのは、timeoutされる処理の中でExceptionをrescueしてtimeoutを起こさないようにしていた場合ですが、あるとしたらむしろ意図しないバグだろうと考えました。
前者はまさにその一例ですね。

前者がバグであることについては完全に同意します

[nobu (Nobuyoshi Nakada)]

(13/09/05 0:25), KOSAKI Motohiro wrote:

次に出てくるのは逆に、デーモンなどで全部捕まえたいケースで、rescue Timeout::Error, StandardError =>e
と書きましょう（書いている）という記述 http://d.hatena.ne.jp/dreammind/20090217/1234813224

２番めは動かなくなりますよね。

Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。

いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。

そのblogにあるコードでは、timeoutが発生するのはnet/httpの中、捕まえるのは外なので、今回の変更による影響はないということです。
StandardErrorで捕まえられるか否かはこれとはまた別件です。

それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、（ログを吐くなどした後）もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。

なので、副作用のほうが気になるわけです。

Exceptionで丸ごと握りつぶしてしまうのがまずいことはまったく同意ですが、完全ではないにせよそれを避けられるほうがいいのではないでしょうか。

[kosaki (Motohiro KOSAKI)]

(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/1234813224

２番めは動かなくなりますよね。

Timeout.timeoutから抜けるときには(指定がなければ)Timeout::Errorを投げるので、影響はないですね。

いや、それは blogの誤記だと思いますよ。StandardErrorで捕まえられないエラーがあるからコード変えたという記事なので。

そのblogにあるコードでは、timeoutが発生するのはnet/httpの中、捕まえるのは外なので、今回の変更による影響はないということです。
StandardErrorで捕まえられるか否かはこれとはまた別件です。

なるほど

それに Exception で捕まえてしまうと、SignalExceptionとかも食ってしまうので
一端は捕まえるけど、（ログを吐くなどした後）もう一度raiseするというコードを
書いてない限りは、どの道遅かれ早かれプログラムは誤動作するので、
この修正では問題は解決してないのです。

なので、副作用のほうが気になるわけです。

Exceptionで丸ごと握りつぶしてしまうのがまずいことはまったく同意ですが、完全ではないにせよそれを避けられるほうがいいのではないでしょうか。

趣旨はだいたい分かりました。
では NEWSに追記だけしてもらえますか。これで壊れるコードがないかは依然として不明ですが、リスクは低いと思うので
アプリケーション側を直してもらうというのでいいような気がしてきました

[takiuchi (Genki Takiuchi)]

そもそも rescue Exceptionを使うのはまずいという意見が出ていますが、
例えば以下ようにフィルタ的に全てのexceptionを補足してそのままraiseするような
利用をしたい場合があり、実際に使われています。

begin
  do_something
rescue Exception => e
  log_exception e
  raise e
end

libraryコードの中でこのようなコードが一箇所でもあると、大外からtimeoutを掛ける事が
できないので、やはり従前の挙動は問題が多いと思います。

[nobu (Nobuyoshi Nakada)]

そういう場合は

  begin
    do_something
  ensure
    log_exception $!
  end

のほうがいいんじゃないかと。

[nobu (Nobuyoshi Nakada)]

すいません、常にログしてはだめですね。

  log_exception $! if $!

[byroot (Jean Boussier)]

• Related to Misc #19740: Block taking methods can't differentiate between a non-local return and a throw added