Project

General

Profile

Actions

Feature #8726

closed

Class#source_location

Added by takiuchi (Genki Takiuchi) over 10 years ago. Updated over 10 years ago.

Status:
Rejected
Assignee:
-
Target version:
[ruby-dev:47569]

Description

Classオブジェクトが生成された場所を返す Class#source_location メソッドの実装を希望いたします。

これによって解決される問題の例としては、Timeout::timeout が無名の例外クラスオブジェクトを raise した
場合に、どこで仕掛けられた timeout なのか、発生場所を特定できるようになります。
このようなケースでは、例外オブジェクトが保有している backtrace はtimeoutが発生した時点での
プログラム実行位置を起点としており、Timeout::timeout(...) を仕掛けた場所の情報は失われています。

Updated by takiuchi (Genki Takiuchi) over 10 years ago

関連して、無名クラスオブジェクトのto_s の結果として、"#<Class:0x007f956336abc8>" のような文字列ではなく、
Class#source_location で得られる情報を付加すれば、様々な利便性の向上が望めると思います。

Updated by naruse (Yui NARUSE) over 10 years ago

ユースケースがtimeoutだけなのでしたら、一般化したsource_locationではなく、
その無名クラスオブジェクトに作成時のcallerを返す特異メソッドをつけた方が早くありませんか。

Updated by naruse (Yui NARUSE) over 10 years ago

naruse (Yui NARUSE) wrote:

ユースケースがtimeoutだけなのでしたら、一般化したsource_locationではなく、
その無名クラスオブジェクトに作成時のcallerを返す特異メソッドをつけた方が早くありませんか。

いや、そもそもTimeout.timeoutのrescueで自身のbacktraceを除いている処理が実はいらなくて、
ちゃんと残しておくべきなのかも。

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

Classオブジェクトが生成された場所を返す Class#source_location メソッドの実装を希望いたします。

これによって解決される問題の例としては、Timeout::timeout が無名の例外クラスオブジェクトを raise した
場合に、どこで仕掛けられた timeout なのか、発生場所を特定できるようになります。
このようなケースでは、例外オブジェクトが保有している backtrace はtimeoutが発生した時点での
プログラム実行位置を起点としており、Timeout::timeout(...) を仕掛けた場所の情報は失われています。

言ってることが今ひとつ理解できません。

test.rb

1: require "timeout"
2:
3: def foo
4:   sleep 10
5: end
6:
7: Timeout.timeout(1) {
8:   foo
9: }

というコードの結果は

% ./ruby-trunk ../test.rb
../test.rb:4:in `sleep': execution expired (Timeout::Error)
from ../test.rb:4:in `foo'
from ../test.rb:8:in `block in <main>'
from ../test.rb:7:in `<main>'

であり、タイムアウトが発生した時点でのプログラム実行位置(foo内の行番号4)と
Timeout仕掛けた場所(行番号7から9のブロック)は両方共わかるように見えます。

なるせさん、わたし、あのバックトレースの整形処理がイマイチ理解できんのだが、
あれはどういう処理を意図してるの。

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

なるせさん、わたし、あのバックトレースの整形処理がイマイチ理解できんのだが、
あれはどういう処理を意図してるの。

printfデバッグした感じだと

   (bt = e.backtrace).reject! {|m| rej =~ m}

の行で取り除かれてる行はないように見える。うーん、コード履歴を追う必要があるかなあ

Updated by nobu (Nobuyoshi Nakada) over 10 years ago

(13/08/04 6:33), KOSAKI Motohiro wrote:

なるせさん、わたし、あのバックトレースの整形処理がイマイチ理解できんのだが、
あれはどういう処理を意図してるの。

printfデバッグした感じだと

  (bt = e.backtrace).reject! {|m| rej =~ m}

の行で取り除かれてる行はないように見える。うーん、コード履歴を追う必要があるかなあ

そこで取り除いているのは、Thread#raise用のタイマースレッドの終了待ちの行です。
timeout.rbからのバックトレースを取り除いているのは、その後のwhileループ中のbt.delete_atです。


--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
中田 伸悦

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

2013/8/3 Nobuyoshi Nakada :

(13/08/04 6:33), KOSAKI Motohiro wrote:

なるせさん、わたし、あのバックトレースの整形処理がイマイチ理解できんのだが、
あれはどういう処理を意図してるの。

printfデバッグした感じだと

  (bt = e.backtrace).reject! {|m| rej =~ m}

の行で取り除かれてる行はないように見える。うーん、コード履歴を追う必要があるかなあ

そこで取り除いているのは、Thread#raise用のタイマースレッドの終了待ちの行です。
timeout.rbからのバックトレースを取り除いているのは、その後のwhileループ中のbt.delete_atです。

やってみました? bt.delete_atのほうは意図通り動いてそうだからコメントしなかったのだけど、こっちのrejectは

% ./ruby-trunk ../test.rb
/home/kosaki/local/ruby-trunk/lib/ruby/2.1.0/timeout.rb:70:in `join':
execution expired (Timeout::Error)
from ../test.rb:7:in `<main>'

と、現在は仕様としてファイル名行番号の後ろに in `join' がついているため、
\z が邪魔してマッチしていません。

\zを取ると以下のように変化しこちらが意図した結果ではないかと

% ./ruby-trunk ../test.rb
../test.rb:7:in `<main>': execution expired (Timeout::Error)

ちょっと最初の説明が足りなかったか。

ただ、話をもとに戻すと、なるせさんの、そもそもなんで無理やり整形して情報量落とす必要あるのだという指摘は至極まっとうなものに思えます。

前回のメールのスクリプトをruby-1.8.7で実行すると

% /usr/bin/ruby ../test.rb
/usr/lib/ruby/1.8/timeout.rb:64:in `foo': execution expired (Timeout::Error)
from ../test.rb:8
from ../test.rb:7

timeout.rb という文字列が見えているので1.9からの変更だと思うのですが、
こう変えるメリットがちと想像つかぬ

Updated by ko1 (Koichi Sasada) over 10 years ago

  • Category set to core

ちょっと追えていないのですが、元の提案自体は reject という感じでしょうか。

Updated by naruse (Yui NARUSE) over 10 years ago

ko1 (Koichi Sasada) wrote:

ちょっと追えていないのですが、元の提案自体は reject という感じでしょうか。

Class#source_location が欲しい状況って言うのはあり得るとは思いますが、
少なくとも timeout の件はユースケースとして妥当ではないように感じました。

Class#source_location 提案を粘るか、timeout 改善に切り替えるかはお任せしますが、
timeout 改善を本格的に議論するならタイトルと違うので別チケットかもしれませんね。

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

  • Status changed from Open to Rejected

とりあえず一回Rejectするので、諦めてない人は理由を整理して再チャレンジしてください。
Timeoutが意図通り動いてない(フィルターでちゃんとフィルターされてない)は、バグだと思うのでこちらで直しておきます。
Timeoutでフィルターするのを外したらどうか、という提案は別件だと思うので必要と思う人が別チケットをオープンしてください。

Updated by takiuchi (Genki Takiuchi) over 10 years ago

Timeoutのバグの本質は、timeoutブロック内で rescue Exception した場合に、timeoutの実装が
内部的に使っている Timeout::ExitExceptionの無名派生クラスを拾ってしまうことのようです。

Timeoutの例は一例でして、Class#source_location があれば無名クラスがどこで定義されたものか
デバッグするのが容易になる、というのが趣旨でした。

Updated by kosaki (Motohiro KOSAKI) over 10 years ago

Timeoutのバグの本質は、timeoutブロック内で rescue Exception した場合に、timeoutの実装が
内部的に使っている Timeout::ExitException を拾ってしまうことのようです。

Timeoutの例は一例でして、Class.source_location があれば無名クラスがどこで定義されたものか
デバッグするのが容易になる、というのが趣旨でした。

これ、整理度合いが進化してませんよね?
Timeoutの問題ならTimeoutを直さないのはなぜなのか。Genericなデバッグメソッドを追加するなら、
それがTimeout以外の一般的な話として有用であることを示せるか。というのが論点だったはずで、
なんら追加がないように見えます。

Updated by takiuchi (Genki Takiuchi) over 10 years ago

了解です。Timeoutのバグは別なissueにしますね。

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0