Project

General

Profile

Bug #4223

GC.stress = true で謎の ArgumentError

Added by metanest (Makoto Kishimoto) almost 9 years ago. Updated over 8 years ago.

Status:
Closed
Priority:
Normal
Target version:
ruby -v:
-
Backport:
[ruby-dev:42907]

Description

=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


Files

patch-bug4223.txt (253 Bytes) patch-bug4223.txt metanest (Makoto Kishimoto), 07/07/2011 09:39 PM
patch-bug4223.txt (611 Bytes) patch-bug4223.txt metanest (Makoto Kishimoto), 07/13/2011 09:48 AM

Associated revisions

Revision 4b29cccf
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@32551 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 32551
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

Revision 2cdfa81a
Added by mrkn (Kenta Murata) over 8 years ago

  • 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.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/branches/ruby_1_9_3@32552 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

History

Updated by naruse (Yui NARUSE) over 8 years ago

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

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

Updated by metanest (Makoto Kishimoto) over 8 years ago

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

$ 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
'

Updated by nahi (Hiroshi Nakamura) over 8 years ago

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

Updated by naruse (Yui NARUSE) over 8 years ago

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

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

Updated by metanest (Makoto Kishimoto) over 8 years ago

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

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

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

Updated by mrkn (Kenta Murata) over 8 years ago

  • 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)

Updated by mrkn (Kenta Murata) over 8 years ago

むらたです。

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)

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

2011年7月8日19:34 Kenta Murata muraken@gmail.com:

むらたです。

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としかいいようがない。

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

2011年7月8日19:34 Kenta Murata muraken@gmail.com:

むらたです。

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としかいいようがない。

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

  • Priority changed from 3 to Normal

Updated by mrkn (Kenta Murata) over 8 years ago

むらたです。

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)

Updated by mrkn (Kenta Murata) over 8 years ago

むらたです。

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)

Updated by metanest (Makoto Kishimoto) over 8 years ago

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

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

Updated by kosaki (Motohiro KOSAKI) over 8 years ago

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

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

Updated by mrkn (Kenta Murata) over 8 years ago

はい。やっておきます。

#17

Updated by mrkn (Kenta Murata) over 8 years ago

  • 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.

Updated by metanest (Makoto Kishimoto) over 8 years ago

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

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

Also available in: Atom PDF