Bug #7638

trunk で rails の activesupport のテストが失敗してしまう

Added by Hiroshi SHIBATA about 2 years ago. Updated about 2 years ago.

[ruby-dev:46820]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:ruby 2.0.0dev (2012-12-29 trunk 38656) [x86_64-darwin12.2.1] Backport:

Description

Ruby のバグとは言い切れませんが、ある時期から activesupport のテストが落ちるようになってしまいました。

Ruby と Rails のどちらを直すべきか判断できないので、何か教えて頂けると助かります。

落ちてしまうテストは

https://github.com/rails/rails/blob/master/activesupport/test/core_ext/marshal_test.rb

です。テストの実行方法は以下の通りです。

% git clone git://github.com/rails/rails.git
% cd rails
% gem install bundler --pre
% bundle
% cd activesupport
% bundle exec ruby -Itest:lib test/core_ext/marshal_test.rb

テストを実行すると Thread の deadlock が発生しますが、Thread.abort_on_exception = true で実行すると

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/testing/isolation.rb#L31

の method_missing 部分で marshal_load が @calls に追加され、この @calls が L26 で send されて NoMethodError で落ちていることがわかりました。

いつから落ち始めたかを調べたところ、

http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=38175

の変更によって、_dump が @calls に追加されるようになり、rails のテストが落ちはじめ

http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=38568

の変更で _dump ではなく marshal_load が @calls に追加されるようになって今も落ち続けているという状態です。

Rails のコードに marshal_load と marshal_dump を定義すればテストは落ちなくなるのですが、38175 より前では
発生していなかった現象が発生するようになった原因など教えて頂けないでしょうか。


Related issues

Related to Ruby trunk - Bug #7564: r38175 introduces incompatibility Closed 12/15/2012

History

#1 Updated by Tomoyuki Chikanaga about 2 years ago

  • Status changed from Open to Assigned
  • Assignee set to Nobuyoshi Nakada

柴田さん、いつもご報告ありがとうございます。

わたしの理解では r38175rb_obj_respond_to()rb_funcall2() で「respond_to?(mid, true) でメソッドが呼べるかチェックしてから呼ぶ」という処理を rb_check_funcall() という1つの関数でまとめるようにしたというものだと思います。以下のようにすると確かに marshal_dump_dump が呼ばれてその結果 method_missing に渡されています。(rails のテストケースとは違い、method_missingsuper を呼んでNoMethodError を発生させています)

class A
  def method_missing(m, *a)
    @call ||= []
    @call << [m, a]
  end
  attr_reader :call
end
a = A.new
Marshal.dump(a)
p a.call # => [[:marshal_dump, []], [:_dump, [-2]]]

rb_obj_respond_to() でのチェックはほぼ Ruby で書くと obj.respond_to?(m) && obj.send(m) と同様のことをしますが、check_funcall_respond_to()respond_to? が再定義されていない場合には常に真を返していて、その後 check_funcall_callable() で実際にメソッドの定義の有無や可視性のチェックをして、呼べなかったら method_missing を呼ぶ(NoMethodErrorrescue つきで)という挙動をするため、method_missing が呼ばれます。

marshal_dumprespond_to?(:marshal_dump, true)false を返すのに呼ばれているのは確かに意外に感じます。ただこういう挙動は暗黙の型変換で to_int などが呼ばれる時にも同様に method_missing が呼ばれており、Marshal もどのような暗黙のメソッド呼び出しの挙動に揃えられただけという考えかたもできると思います。

(上の A の定義は略します)

a = A.new
Integer(a) rescue nil;
p a.call # => [[:to_int, []], [:to_i, []]]

なので一貫性を取るか互換性を取るかということではないかと思います。
確かに挙動は変化していますが、個人的な意見としては method_missing を呼ばなかったら呼ばなかったで暗黙に呼ばれるメソッドが method_missing を呼ばないぞって文句が出てくるような気もします。
ただ respond_to? が再定義されていると今度は method_missing が呼ばれなくなってしまいます。
上の例だと

class A
  def respond_to?(m, priv=false)
    super
  end
end

が追加されると method_missing が呼ばれなくなります。 respond_to? の再定義というのも筋が悪いと思いますけど respond_to_missing? がまだ普及していなくて時々 gems パッケージのソースで見掛けた記憶があります。

あと method_missing に渡されたメソッドが常に result に対して呼べるつもりで書かれている activesupport のテストはいずれにしても直すべきじゃないかと思います。

#2 Updated by Hiroshi SHIBATA about 2 years ago

nagachika さん、丁寧な解説ありがとうございます。

#7564 の流れをみると respond_to_missing? を使って method_missing に反応すべきでないものは除外した方が良いということなので、その方向で考えてみます。

#3 Updated by Nobuyoshi Nakada about 2 years ago

  • Status changed from Assigned to Closed

Also available in: Atom PDF