Project

General

Profile

Bug #13107

def_delegators causes random errors in MRI 2.4.0

Added by twalpole@gmail.com (Thomas Walpole) over 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:78981]

Description

In the Capybara project we use the rack_test gem which uses def_delegators to forward flast_reponse to @rack_mock_session - https://github.com/brynary/rack-test/blob/master/lib/rack/test.rb#L29

When running the Capybara test test suite calling last_reponse on a rack_test Session object will sporadically result in a TypeError from forwardable.rb

TypeError:
       wrong argument type Integer (expected Proc)

Patching around the delegator with

class Rack::Test::Session
  def last_response
    @rack_mock_session.last_response
  end
end

stops the error from occurring, so it appears that something def_delegators is doing is causing problems. Unfortunately I have not yet been able to establish exactly what causes the issues to produce a simple test case.


Files

Associated revisions

Revision ff3496b0
Added by nobu (Nobuyoshi Nakada) over 2 years ago

vm_insnhelper.c: block argument at tailcall

  • vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts after set up the new frame, not the passed block to be clobbered by invoked finalizers and so on. [ruby-core:78981] [Bug #13107]

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

Revision 57293
Added by nobu (Nobuyoshi Nakada) over 2 years ago

vm_insnhelper.c: block argument at tailcall

  • vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts after set up the new frame, not the passed block to be clobbered by invoked finalizers and so on. [ruby-core:78981] [Bug #13107]

Revision 57293
Added by nobu (Nobuyoshi Nakada) over 2 years ago

vm_insnhelper.c: block argument at tailcall

  • vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts after set up the new frame, not the passed block to be clobbered by invoked finalizers and so on. [ruby-core:78981] [Bug #13107]

Revision 57293
Added by nobu (Nobuyoshi Nakada) over 2 years ago

vm_insnhelper.c: block argument at tailcall

  • vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts after set up the new frame, not the passed block to be clobbered by invoked finalizers and so on. [ruby-core:78981] [Bug #13107]

Revision 66487605
Added by naruse (Yui NARUSE) about 2 years ago

merge revision(s) 57293: [Backport #13107]

    vm_insnhelper.c: block argument at tailcall

    * vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts
      after set up the new frame, not the passed block to be clobbered
      by invoked finalizers and so on.  [ruby-core:78981] [Bug #13107]

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

Revision 57853
Added by naruse (Yui NARUSE) about 2 years ago

merge revision(s) 57293: [Backport #13107]

vm_insnhelper.c: block argument at tailcall

* vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts
  after set up the new frame, not the passed block to be clobbered
  by invoked finalizers and so on.  [ruby-core:78981] [Bug #13107]

History

Updated by twalpole@gmail.com (Thomas Walpole) over 2 years ago

Forgot to add the location reported for the error

/home/travis/.rvm/rubies/ruby-2.4.0/lib/ruby/2.4.0/forwardable.rb:228:in `last_response'

Updated by eugeneius (Eugene Kenny) over 2 years ago

The mongo gem is also affected by this bug (https://jira.mongodb.org/browse/RUBY-1191). Running the tests in the spec/mongo/grid/stream/read_spec.rb file reliably reproduces it.

I bisected the changes to forwardable.rb between v2.3.0 and v2.4.0, and determined that the bug was introduced in r55376.

I've attached a patch that fixes the problem, although it effectively reverts the optimisation from r55376, so there may be a performance impact.

Updated by eugeneius (Eugene Kenny) over 2 years ago

Here's another patch that also fixes the problem, while keeping the optimisation from r55376.

I'll admit that I have no idea what this option does or how it could cause this bug - I noticed that the "portable" implementation of forwardable/impl.rb wasn't affected, and fiddled with the MRI-specific one until it worked. Please review this patch carefully if you intend to use it!

Removing tailcall_optimization also appears to work, although I think we need that option to get a sensible stack trace. Maybe trace_instruction: false, tailcall_optimization: true is an invalid combination?

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Status changed from Open to Feedback

Enabling trace instruction disables tail call optimization, too.
It seems like a bug of the optimization and GC.
How can I reproduce it briefly?

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Description updated (diff)
#6

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Status changed from Feedback to Closed

Applied in changeset r57293.


vm_insnhelper.c: block argument at tailcall

  • vm_insnhelper.c (vm_call_iseq_setup_tailcall): check interrupts after set up the new frame, not the passed block to be clobbered by invoked finalizers and so on. [ruby-core:78981] [Bug #13107]

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

When interrupted by a finalizer during setting up tailcall frame, a block argument on the stack was overwritten by the magic number in the finalizer frame which looks an Integer.
I tried in vain to make a test case, but a failure in mongo-ruby-driver seems fixed.
Could you try the latest trunk?

Updated by twalpole@gmail.com (Thomas Walpole) over 2 years ago

This appears to be fixed in latest trunk now. Is there any info on when a 2.4.1 will be released - seems like a pretty serious issue.

Updated by vo.x (Vit Ondruch) over 2 years ago

  • Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: REQUIRED

Updated by vo.x (Vit Ondruch) over 2 years ago

Just FTR, this is causing failures in Mongo gem test suite [1]. Applying this patch fixed the issues.

[1] https://jira.mongodb.org/browse/RUBY-1194

Updated by vo.x (Vit Ondruch) over 2 years ago

And I can confirm that also rack-test test suite is passing with this patch [1].

[1] https://apps.fedoraproject.org/koschei/package/rubygem-rack-test?collection=f26

Updated by naruse (Yui NARUSE) about 2 years ago

  • Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: REQUIRED to 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: DONE

ruby_2_4 r57853 merged revision(s) 57293.

Updated by Benoit_Tigeot (Benoit Tigeot) about 2 years ago

twalpole@gmail.com (Thomas Walpole) wrote:

This appears to be fixed in latest trunk now. Is there any info on when a 2.4.1 will be released - seems like a pretty serious issue.

I am in the same situation actually. Locked because of this issue. Any idea when 2.4.1 will be released?

Also available in: Atom PDF