[Ruby 1.9 - Bug #5152][Open] TestDateNew#test civil が GC.stress = true 下で Failure

Ruby Development
1

Issue #5152 has been reported by Tomoyuki C…

Bug #5152: TestDateNew#test_civil が GC.stress = true 下で Failure

Author: Tomoyuki C.
Status: Open
Priority: Normal
Assignee: tadayoshi funaba
Category: ext
Target version: 1.9.3
ruby -v: ruby 1.9.4dev (2011-08-03 trunk 32823) [x86_64-linux]

make test-all TESTS=“-vq --gc-stress date/test_date_new.rb -n
test_civil”
のように GC.stress = true 下で DateTime のテストが Failure します。

  1. Failure:
    test_civil(TestDateNew)
    [/home/chikanaga/opt/ruby-trunk/src/ruby/test/date/test_date_new.rb:100]:
    <[-4712, 1, 1, 0, 0, 0, (3/8)]> expected but was
    <[-4712, 1, 1, 0, 0, 0, (0/1)]>.

以下のようなスクリプトでも再現します。また ruby_1_9_3 でも再現しました。

$ cat datetime.rb
require “date”
GC.stress = true
dt = DateTime.civil(-4712,1,1, 0,0,0, ‘+0900’)
p dt.offset # => (3/8) になるはずが (0/1) になる

date_parse.c の date_zone_to_diff での str の GC 保護されていないためだと思います。
以下のような変更で期待した offset を得られるようになりました。

diff --git a/ext/date/date_parse.c b/ext/date/date_parse.c
index 3605ff7…e41606b 100644
— a/ext/date/date_parse.c
+++ b/ext/date/date_parse.c
@@ -374,6 +374,7 @@ date_zone_to_diff(VALUE str)
sp = 0;
}
}

  • RB_GC_GUARD(str);
    if (d > dest) {
    if (*(d - 1) == ’ ')
    –d;
2

Issue #5152 has been updated by Tomoyuki C…

すみません、ソースコードを良く読むと RB_GC_GUARD()
を挿入しないといけないのはさっきのところじゃなくて関数の最後のほうに挿入する必要がありそうでした。

また parse_ddd_cb() にも GC 保護が必要な変数がありそうでした。

@@ -1323,6 +1324,7 @@ parse_ddd_cb(VALUE m, VALUE hash)
}
break;
}

  • RB_GC_GUARD(s2);
    if (!NIL_P(s3)) {
    cs3 = RSTRING_PTR(s3);
    l3 = RSTRING_LEN(s3);
    @@ -1354,6 +1356,7 @@ parse_ddd_cb(VALUE m, VALUE hash)
    }
    }
    }
  • RB_GC_GUARD(s3);
    if (!NIL_P(s4)) {
    l4 = RSTRING_LEN(s4);

@@ -1392,6 +1395,7 @@ parse_ddd_cb(VALUE m, VALUE hash)
set_hash(“offset”, date_zone_to_diff(rb_str_new2(s1)));
}
}

  • RB_GC_GUARD(s5);

    return 1;
    }

Bug #5152: TestDateNew#test_civil が GC.stress = true 下で Failure

Author: Tomoyuki C.
Status: Open
Priority: Normal
Assignee: tadayoshi funaba
Category: ext
Target version: 1.9.3
ruby -v: ruby 1.9.4dev (2011-08-03 trunk 32823) [x86_64-linux]

make test-all TESTS=“-vq --gc-stress date/test_date_new.rb -n
test_civil”
のように GC.stress = true 下で DateTime のテストが Failure します。

  1. Failure:
    test_civil(TestDateNew)
    [/home/chikanaga/opt/ruby-trunk/src/ruby/test/date/test_date_new.rb:100]:
    <[-4712, 1, 1, 0, 0, 0, (3/8)]> expected but was
    <[-4712, 1, 1, 0, 0, 0, (0/1)]>.

以下のようなスクリプトでも再現します。また ruby_1_9_3 でも再現しました。

$ cat datetime.rb
require “date”
GC.stress = true
dt = DateTime.civil(-4712,1,1, 0,0,0, ‘+0900’)
p dt.offset # => (3/8) になるはずが (0/1) になる

date_parse.c の date_zone_to_diff での str の GC 保護されていないためだと思います。
以下のような変更で期待した offset を得られるようになりました。

diff --git a/ext/date/date_parse.c b/ext/date/date_parse.c
index 3605ff7…e41606b 100644
— a/ext/date/date_parse.c
+++ b/ext/date/date_parse.c
@@ -374,6 +374,7 @@ date_zone_to_diff(VALUE str)
sp = 0;
}
}

  • RB_GC_GUARD(str);
    if (d > dest) {
    if (*(d - 1) == ’ ')
    –d;
3

Issue #5152 has been updated by Tomoyuki C…

了解しました。
今週は追加の確認ができそうにないので、とりあえず現在わかっているところを入れてしまいます。

Bug #5152: TestDateNew#test_civil が GC.stress = true 下で Failure

Author: Tomoyuki C.
Status: Open
Priority: Normal
Assignee: tadayoshi funaba
Category: ext
Target version: 1.9.3
ruby -v: -

make test-all TESTS=“-vq --gc-stress date/test_date_new.rb -n
test_civil”
のように GC.stress = true 下で DateTime のテストが Failure します。

  1. Failure:
    test_civil(TestDateNew)
    [/home/chikanaga/opt/ruby-trunk/src/ruby/test/date/test_date_new.rb:100]:
    <[-4712, 1, 1, 0, 0, 0, (3/8)]> expected but was
    <[-4712, 1, 1, 0, 0, 0, (0/1)]>.

以下のようなスクリプトでも再現します。また ruby_1_9_3 でも再現しました。

$ cat datetime.rb
require “date”
GC.stress = true
dt = DateTime.civil(-4712,1,1, 0,0,0, ‘+0900’)
p dt.offset # => (3/8) になるはずが (0/1) になる

date_parse.c の date_zone_to_diff での str の GC 保護されていないためだと思います。
以下のような変更で期待した offset を得られるようになりました。

diff --git a/ext/date/date_parse.c b/ext/date/date_parse.c
index 3605ff7…e41606b 100644
— a/ext/date/date_parse.c
+++ b/ext/date/date_parse.c
@@ -374,6 +374,7 @@ date_zone_to_diff(VALUE str)
sp = 0;
}
}

  • RB_GC_GUARD(str);
    if (d > dest) {
    if (*(d - 1) == ’ ')
    –d;
4

$B$H$j$"$($:%3%_%C%H$7$F$*$$$F2<$5$$!#(B