Block doesn't retain it's binding through instance_eval #4478

Closed
sofaking opened this Issue Feb 6, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@sofaking

sofaking commented Feb 6, 2017

module Outer
  def foo(&block)
    Inner.new.plain_method {}
  end

  def bar(&block)
    Inner.new(&block)
  end

  class Inner
    def initialize(&block)
      @block = block

      instance_exec(&block) if block_given?
    end

    def plain_method(&block)
      block.binding.receiver.to_s
    end

    def method_missing(method_name, *args)
      @block.binding.receiver.to_s
    end
  end
end

class Extender
  extend Outer

  puts foo {}
  bar do
    puts inexisting_method 'abc'
  end
end

Environment

  • jruby 9.1.7.0 (2.3.1) 2017-01-11 68056ae Java HotSpot(TM) 64-Bit Server VM 25.102-b14 on 1.8.0_102-b14 +jit [darwin-x86_64]
  • Darwin Antons-MacBook-Pro.local 16.4.0 Darwin Kernel Version 16.4.0: Thu Dec 22 22:53:21 PST 2016; root:xnu-3789.41.3~3/RELEASE_X86_64 x86_64

Expected Behavior

I expect this code to return 'Extender' 2 times, in the same manner as MRI does.

Actual Behavior

For some reason, after instance_eval a passed block looses it's binding, and becomes bound to the object it was instance_eval-ed. So the actual result is

Extender
#<Outer::Inner:0x4923ab24>

Sorry, probably I could come up with a simpler example. I've just roughly cropped my real application to the smallest possible size.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 8, 2017

Member

Yep, this is a bug. I've long known our instance_eval permanently modifies the incoming block, but we had no clear example of failures that would cause. Kudos!

This should be fixable by cloning more of the block. This will impact the performance of instance_eval (more to clone, more to clean up) but we may be able to do it only for blocks that have been reified into procs (since literal blocks are only alive for the duration of the instance_eval call).

Member

headius commented Feb 8, 2017

Yep, this is a bug. I've long known our instance_eval permanently modifies the incoming block, but we had no clear example of failures that would cause. Kudos!

This should be fixable by cloning more of the block. This will impact the performance of instance_eval (more to clone, more to clean up) but we may be able to do it only for blocks that have been reified into procs (since literal blocks are only alive for the duration of the instance_eval call).

@headius headius added this to the JRuby 9.1.8.0 milestone Feb 8, 2017

@headius headius added the core label Feb 8, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 8, 2017

Member

FWIW this is a much better repro than we usually get. We appreciate the extra effort to narrow this down.

Member

headius commented Feb 8, 2017

FWIW this is a much better repro than we usually get. We appreciate the extra effort to narrow this down.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 9, 2017

Member

Ahh boo, I pushed #4485 for this but forgot to tag this issue. If it looks ok I'll re-commit that change.

Member

headius commented Feb 9, 2017

Ahh boo, I pushed #4485 for this but forgot to tag this issue. If it looks ok I'll re-commit that change.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 9, 2017

Member

Ugh, and #4485 was not even for this bug. Nevermind.

Member

headius commented Feb 9, 2017

Ugh, and #4485 was not even for this bug. Nevermind.

@headius headius closed this in 55b3080 Feb 9, 2017

@headius headius reopened this Feb 9, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Feb 9, 2017

Member

Oops.

Member

headius commented Feb 9, 2017

Oops.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 2, 2017

Member

Well I'm not sure if it was 55b3080, but this appears to be fixed on master.

~/projects/jruby $ jruby blah.rb
Extender
Extender
Member

headius commented Mar 2, 2017

Well I'm not sure if it was 55b3080, but this appears to be fixed on master.

~/projects/jruby $ jruby blah.rb
Extender
Extender

@headius headius closed this Mar 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment