Bug #4223: GC.stress = true で謎の ArgumentError

• Status: Closed
• Assignee: mrkn (Kenta Murata)
• Target version: 1.9.3
• ruby -v: -

[metanest (Makoto Kishimoto)]

=begin
手元の環境で、trunk の r29690 以降で、次のようなスクリプトが、

foo.rb

GC.stress = true
t = Time.local(2000)
File.utime(t + 1, t + 2, "foo.rb")

こんな感じに謎の ArgumentError になります

$ ruby19 -v foo.rb
ruby 1.9.3dev (2010-12-29 trunk 30417) [x86_64-freebsd8.2]
foo.rb:4:in utime': time out of system range (ArgumentError) from foo.rb:4:in '
=end

[naruse (Yui NARUSE)]

• Status changed from Open to Feedback
• Assignee set to metanest (Makoto Kishimoto)

現在のtrunkで再現しませんがどうでしょう

[metanest (Makoto Kishimoto)]

私の手元の環境では変わってません。

$ cat foo.rb

foo.rb

GC.stress = true
t = Time.local(2000)
File.utime(t + 1, t + 2, "foo.rb")

$ ruby19 -v foo.rb
ruby 1.9.3dev (2011-06-26 trunk 32231) [x86_64-freebsd8.2]
foo.rb:4:in utime': time out of system range (ArgumentError) from foo.rb:4:in '

[nahi (Hiroshi Nakamura)]

• Status changed from Feedback to Assigned
• Assignee changed from metanest (Makoto Kishimoto) to naruse (Yui NARUSE)
• Target version set to 1.9.3

[naruse (Yui NARUSE)]

• Assignee changed from naruse (Yui NARUSE) to metanest (Makoto Kishimoto)

うーん、UFSでもZFSでも再現しなくて謎なので、再現できる岸本さんが頑張ってください

[metanest (Makoto Kishimoto)]

• File patch-bug4223.txt patch-bug4223.txt added

直りました。パッチ添付します

[kosaki (Motohiro KOSAKI)]

直感的には、関数の最後でRB_GC_GUARD()しないといけない気がしますがどう思います？ ＞ むらけんさん

[mrkn (Kenta Murata)]

• ruby -v changed from ruby 1.9.3dev (2010-12-29 trunk 30417) [x86_64-freebsd8.2] to -

むらたです。

On Thursday, July 7, 2011 at 9:49 PM, Motohiro KOSAKI wrote:

直感的には、関数の最後でRB_GC_GUARD()しないといけない気がしますがどう思います？ ＞ むらけんさん

z = bignew(zn, RBIGNUM_SIGN(x));

ここで GC が走る可能性があって、そういう場合に x をガードする事が目的なんだと思います。
ですから、bigadd_int の最初に RB_GC_GUARD して良いです。

--
Kenta Murata
Sent with Sparrow (http://www.sparrowmailapp.com)

[mrkn (Kenta Murata)]

むらたです。

On Thursday, July 7, 2011 at 9:49 PM, Motohiro KOSAKI wrote:

直感的には、関数の最後でRB_GC_GUARD()しないといけない気がしますがどう思います？ ＞ むらけんさん

z = bignew(zn, RBIGNUM_SIGN(x));

ここで GC が走る可能性があって、そういう場合に x をガードする事が目的なんだと思います。
ですから、bigadd_int の最初に RB_GC_GUARD して良いです。

--
Kenta Murata
Sent with Sparrow (http://www.sparrowmailapp.com)

[kosaki (Motohiro KOSAKI)]

2011年7月8日19:34 Kenta Murata [e-mail]:

むらたです。

On Thursday, July 7, 2011 at 9:49 PM, Motohiro KOSAKI wrote:

直感的には、関数の最後でRB_GC_GUARD()しないといけない気がしますがどう思います？ ＞ むらけんさん

z = bignew(zn, RBIGNUM_SIGN(x));

ここで GC が走る可能性があって、そういう場合に x をガードする事が目的なんだと思います。
ですから、bigadd_int の最初に RB_GC_GUARD して良いです。

あ、すいません。昨日僕のいない間に #ruby-jaで議論して後ろに移動させるという
結論になったみたいです。

22:49:17 途中に付けても無駄になることがある
22:50:09 最後じゃなくてもいいけど、ガードが必要な部分のあとにないとダメ
22:50:41 結局そういう事になったのか
22:51:41 では、私のパッチよりもっとあとでないとまずいですね
22:51:47 経験則的に
22:52:25 前に置いてるの沢山ありますよ たしか。
22:52:41 xdsを参照してる最後の場所より後だから、同じ関数のwhileの後
22:53:15 RB_GC_GUARDをもうちょい工夫すればいいのかなぁ
22:53:27 全く同じ構造のコードだから bigsub_int にも要るかな
22:53:27 [ruby-dev:40942] matzにこういわれたので、
22:53:28 {unak_away} http://mla.n-z.jp/?ruby-dev=40942
22:53:48 VC向けのRB_GC_GUARDは試行錯誤した
22:54:16 フロー解析して最適化されちゃうなら、工夫じゃどうにもならなさそうな
22:54:35 経験的にOKとしかいいようがない。

[kosaki (Motohiro KOSAKI)]

2011年7月8日19:34 Kenta Murata [e-mail]:

むらたです。

On Thursday, July 7, 2011 at 9:49 PM, Motohiro KOSAKI wrote:

直感的には、関数の最後でRB_GC_GUARD()しないといけない気がしますがどう思います？ ＞ むらけんさん

z = bignew(zn, RBIGNUM_SIGN(x));

ここで GC が走る可能性があって、そういう場合に x をガードする事が目的なんだと思います。
ですから、bigadd_int の最初に RB_GC_GUARD して良いです。

あ、すいません。昨日僕のいない間に #ruby-jaで議論して後ろに移動させるという
結論になったみたいです。

22:49:17 途中に付けても無駄になることがある
22:50:09 最後じゃなくてもいいけど、ガードが必要な部分のあとにないとダメ
22:50:41 結局そういう事になったのか
22:51:41 では、私のパッチよりもっとあとでないとまずいですね
22:51:47 経験則的に
22:52:25 前に置いてるの沢山ありますよ たしか。
22:52:41 xdsを参照してる最後の場所より後だから、同じ関数のwhileの後
22:53:15 RB_GC_GUARDをもうちょい工夫すればいいのかなぁ
22:53:27 全く同じ構造のコードだから bigsub_int にも要るかな
22:53:27 [ruby-dev:40942] matzにこういわれたので、
22:53:28 {unak_away} http://mla.n-z.jp/?ruby-dev=40942
22:53:48 VC向けのRB_GC_GUARDは試行錯誤した
22:54:16 フロー解析して最適化されちゃうなら、工夫じゃどうにもならなさそうな
22:54:35 経験的にOKとしかいいようがない。

[kosaki (Motohiro KOSAKI)]

• Priority changed from 3 to Normal

[mrkn (Kenta Murata)]

むらたです。

On Friday, July 8, 2011 at 7:55 PM, KOSAKI Motohiro wrote:

あ、すいません。昨日僕のいない間に #ruby-jaで議論して後ろに移動させるという
結論になったみたいです。

22:49:17 途中に付けても無駄になることがある
22:50:09 最後じゃなくてもいいけど、ガードが必要な部分のあとにないとダメ
22:50:41 結局そういう事になったのか
22:51:41 では、私のパッチよりもっとあとでないとまずいですね
22:51:47 経験則的に
22:52:25 前に置いてるの沢山ありますよ たしか。
22:52:41 xdsを参照してる最後の場所より後だから、同じ関数のwhileの後
22:53:15 RB_GC_GUARDをもうちょい工夫すればいいのかなぁ
22:53:27 全く同じ構造のコードだから bigsub_int にも要るかな
22:53:27 [ruby-dev:40942] matzにこういわれたので、
22:53:28 {unak_away} http://mla.n-z.jp/?ruby-dev=40942
22:53:48 VC向けのRB_GC_GUARDは試行錯誤した
22:54:16 フロー解析して最適化されちゃうなら、工夫じゃどうにもならなさそうな
22:54:35 経験的にOKとしかいいようがない。

なんと、RB_GC_GUARD の意味がコンパイラの進化に伴なって変わってる事を知りませんでした。

おとなしく bigdecimal.c を見なおして来ます orz

Kenta Murata
Sent with Sparrow (http://www.sparrowmailapp.com)

[mrkn (Kenta Murata)]

むらたです。

On Friday, July 8, 2011 at 7:55 PM, KOSAKI Motohiro wrote:

あ、すいません。昨日僕のいない間に #ruby-jaで議論して後ろに移動させるという
結論になったみたいです。

22:49:17 途中に付けても無駄になることがある
22:50:09 最後じゃなくてもいいけど、ガードが必要な部分のあとにないとダメ
22:50:41 結局そういう事になったのか
22:51:41 では、私のパッチよりもっとあとでないとまずいですね
22:51:47 経験則的に
22:52:25 前に置いてるの沢山ありますよ たしか。
22:52:41 xdsを参照してる最後の場所より後だから、同じ関数のwhileの後
22:53:15 RB_GC_GUARDをもうちょい工夫すればいいのかなぁ
22:53:27 全く同じ構造のコードだから bigsub_int にも要るかな
22:53:27 [ruby-dev:40942] matzにこういわれたので、
22:53:28 {unak_away} http://mla.n-z.jp/?ruby-dev=40942
22:53:48 VC向けのRB_GC_GUARDは試行錯誤した
22:54:16 フロー解析して最適化されちゃうなら、工夫じゃどうにもならなさそうな
22:54:35 経験的にOKとしかいいようがない。

なんと、RB_GC_GUARD の意味がコンパイラの進化に伴なって変わってる事を知りませんでした。

おとなしく bigdecimal.c を見なおして来ます orz

Kenta Murata
Sent with Sparrow (http://www.sparrowmailapp.com)

[metanest (Makoto Kishimoto)]

• File patch-bug4223.txt patch-bug4223.txt added

新しいパッチです。
更新点は、

• bigsub_int() も対策した
• RB_GC_GUARD をコントロールフロー的に十分に後ろの場所に移動した
  の 2 点です。
  コミット（とバックポート）とクローズ（でいいですよね）お願いします。

[kosaki (Motohiro KOSAKI)]

• Assignee changed from metanest (Makoto Kishimoto) to mrkn (Kenta Murata)

これは1.9.3 ターゲットのままでいいですよね？ > むらけんさん

[mrkn (Kenta Murata)]

はい。やっておきます。

[mrkn (Kenta Murata)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r32551.
Makoto, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• bignum.c (bigsub_int): add RB_GC_GUARD. This patch is made by
  Makoto Kishimoto. fixes #4223 [ruby-dev:42907]
• bignum.c (bigadd_int): ditto.

[metanest (Makoto Kishimoto)]

RB_GC_GUARD マクロについて簡単にまとめておきます

• オブジェクトの中身を指してるポインタがCコード中にあって参照中なのに、オブジェクトを指すポインタ（VALUE）がどこにもなくなっちゃってGCに回収されるのを防ぐ
• 関数の最初のほうにあるコードがけっこうある。展開内容を工夫して、それでも困らないようにがんばってはいるけど、コンパイラによる高度な最適化で、結構わからない
• ソースコードのコントロールフロー中、オブジェクトの中身にアクセスしていて、かつGCが起きうる場所（rubyコードが動く、オブジェクトを作る）よりも後ろに置くことが、安全のためには必要。将来コードを追加することを考えると、より（コントロールフロー的に）後ろのほうに