FYI. This commit also fixes JRUBY-6705.
----------
commit 7c3f6426eb26e560f21b2f1905923fa624c4408c
Author: Subramanya Sastry <sss.li...@gmail.com>
Date: Thu Jun 21 23:43:39 2012 -0500
HEADS UP! Removed all trace of setWithinDefined and isWithinDefined.
* Tested on rubyspecs in 1.8 and 1.9 mode -- AST, JIT, IR. No
change in failures after this change.
* Removed this from AST interpreter, JIT, IR
* setWithinDefined was set by all of these before entering defined?
checks and cleared on exit. isWithinDefined was only used by
RaiseException to decide whether to set $! or not -- effectively
$! was not being set for exceptions raised by code executing
by the call tree rooted in the defined? checks. However, this
resulted in buggy behavior because this call tree could do a ton
of work (including loading files, requires, setting up classes,
etc.) any of which can raise exceptions and might have to be
handled, which require access to $!. The workaround in IR and
AST interpreters was to set "$!" whenever an exception was
received (effectively bypassing the condition that RaiseException
was guarding). But, since I removed both these hacky checks as
part of a previous commit (20632af6e1fa511fd9b762beda2a60d39bf3c859),
we had code failures in Rails (bug JRUBY-6705) because of files
not getting loaded properly. So, given that isWithinDefined is only
used by RaiseException, which was anyway getting bypassed, there is
no reason to keep that check around. Once I removed that check,
since there was no other use of isWithinDefined in the codebase,
I also removed setWithinDefined, which led to a bunch of dead code
which I purged.
* NOTE: JIT is probably yet to remove the $! hack that I removed in
the other commit. It would be best to remove that hack as well
so we know for sure that these two commits are working together
well without bugs.
--------
On Sun, Jun 17, 2012 at 5:51 PM, Subramanya Sastry <sss.li...@gmail.com>wrote:
> Please refer to bug: https://jira.codehaus.org/browse/JRUBY-6705
>
> I reduced this bug to the following code snippet. My comments and
> questions follow this snippet
>
>
> --------------------------------------------------------------------------------------
> class Object
> def self.const_missing(*args)
> puts "in const_missing! #{args.inspect}"
> raise "this_will_get_lost!"
> rescue Exception => e
> puts "Got exception: #{e}"
> end
> end
>
> begin
> raise "will_not_get_lost_1"
> rescue Exception => e
> puts "Got exception: #{e}"
> end
>
> defined?(Foo::Bar)
>
> begin
> raise "will_not_get_lost_2"
> rescue Exception => e
> puts "Got exception: #{e}"
> end
>
> --------------------------------------------------------------------------------------
>
> If you run this on 1.7 master, you will notice that the exception within
> const_missing will not be printed out in the rescue handler. But, if you
> revert my commit "20632af6e1fa511fd9b762beda2a60d39bf3c859", this code
> snippet will work as expected (unrelated observation: in -X+C mode, the
> const_missing method doesn't run in either case). I think my fixes in
> that commit are "correct" and are in the right direction. I dont think the
> hacky $! set should be around in the interpreter or compiler (AST or IR).
> Those checks were working around lost sets of "$!" when exceptions were
> being raised. I traced one of the instances to RubyThread and fixed it.
>
> But, as part of this bug report, I found a more important one -- where $!
> was not being set by any code that runs as part of an ancestor defined?
> call in the call stack. This bug is caused by the check on line 239 of
> org.jruby.exceptions.RaiseException.java. So, I wanted to check if someone
> knows more about why $! setting is being suppressed when in defined? code.
> Clearly, the interpreter/compiler hack that sets $! to the received
> exception essentially gets around that check. So, it looks like a
> different solution should be possible -- I haven't yet attempted to get rid
> of the check in RaiseException.java and see what happens ... but, before I
> did that, I thought I would check first.
>
> Subbu.
>
