Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rescue clause yielding TypeError hits %undefined #2417

Closed
gregspurrier opened this issue Jan 3, 2015 · 11 comments
Closed

Rescue clause yielding TypeError hits %undefined #2417

gregspurrier opened this issue Jan 3, 2015 · 11 comments

Comments

@gregspurrier
Copy link

@gregspurrier gregspurrier commented Jan 3, 2015

[This may be a duplicate of Issue #2267, but it references jruby-head and I am able to give more detail about the failure mode.]

With the JRuby 1.7.18 release I started seeing the following error when running the Klam specs under Travis CI:

TypeError: class or module required for rescue clause. Found: %undefined

If I down-rev to JRuby 1.7.17, the tests pass. Here are examples of failing and passing builds differing only in the version of JRuby.

The code snippet that triggers the first of the failures is:

(begin; (false ? false : __send__(:"simple-error",'cond failure')); rescue => __KLAM_001; __send__(:"error-to-string",__KLAM_001); end)

This string is generated by the Klam compiler and is being executed via instance_eval within an instance of a subclass of BasicObject.

NOTE: I am not able to reproduce this failure locally under OS X with Java 1.7.0_67.

gregspurrier added a commit to gregspurrier/shen-ruby that referenced this issue Jan 5, 2015
JRuby 1.7.18 fails with an error regarding rescue being used outside of
a module or class. See jruby/jruby#2417.
@headius
Copy link
Member

@headius headius commented Jan 7, 2015

Very weird.

@headius headius added this to the JRuby 1.7.19 milestone Jan 7, 2015
@deepj
Copy link

@deepj deepj commented Jan 16, 2015

The same problem is on JRuby 9k too. I met with this issue in case of trying to pass tests of lotus-router on JRuby 9k.

git clone git@github.com:lotus/router.git lotus-router
cd lotus-router
bundle
rake test

@ntl
Copy link

@ntl ntl commented Jan 18, 2015

Just stumbled on this one with jruby-9000-dev built yesterday. Inheriting from Object is fine, but any object that raises an error within a BasicObject subclass dies.

@enebo
Copy link
Member

@enebo enebo commented Jan 18, 2015

Ah yeah this is not an issue with 1.7.18 and is a problem with master/9k. #2267 is causing people to run specs against the wrong version of JRuby. I will update this issue and mark it against 9k.

@enebo enebo added this to the JRuby 9.0.0.0-pre1 milestone Jan 18, 2015
@enebo enebo removed this from the JRuby 1.7.19 milestone Jan 18, 2015
@enebo
Copy link
Member

@enebo enebo commented Jan 18, 2015

@ntl do you happen to have a reduces snippet for this? My repros of making an object inherited from BasicObject so far have not been blowing up.

@enebo enebo changed the title Rescue clause yielding TypeError in JRuby 1.7.18 Rescue clause yielding TypeError hits %undefined Jan 18, 2015
@ntl
Copy link

@ntl ntl commented Jan 18, 2015

This will do it for me. I think you have to rescue an error within BasicObject.

class ExposesJRuby2417 < BasicObject                                               
  def blow_up                                                                      
    begin                                                                          
      ::Kernel.raise "error"                                                       
    rescue                                                                         
    end                                                                            
  end                                                                              
end                                                                                
ExposesJRuby2417.new.blow_up                                                       

@enebo
Copy link
Member

@enebo enebo commented Jan 18, 2015

@ntl Excellent! About as minimal as it probably gets...

@ntl
Copy link

@ntl ntl commented Jan 18, 2015

@enebo glad it's helpful. I strongly resisted the urge to golf it down further :)

@subbuss
Copy link
Contributor

@subbuss subbuss commented Jan 19, 2015

The problem is that 'StandardError' is not being found in the inheritance hierarchy when the class inherits BasicObject. So, maybe the bug is that in IR mode, we shouldn't be generating a lookup for StandardError but have it around. See the related FIXME in IRBuilder which talks of a different issue with having the class looked up. I think dealing with the FIXME will fix the issue. But, before I go ahead, it would be good to get some other eyes on it ( @enebo @headius ).


        // SSS FIXME:
        // rescue => e AND rescue implicitly EQQ the exception object with StandardError
        // We generate explicit IR for this test here.  But, this can lead to inconsistent
        // behavior (when compared to MRI) in certain scenarios.  See example:
        //
        //   self.class.const_set(:StandardError, 1)
        //   begin; raise TypeError.new; rescue; puts "AHA"; end
        //
        // MRI rescues the error, but we will raise an exception because of reassignment
        // of StandardError.  I am ignoring this for now and treating this as undefined behavior.
        //
        // Solution: Create a 'StandardError' operand type to eliminate this.
        Variable v = addResultInstr(s, new InheritanceSearchConstInstr(s.createTemporaryVariable(), s.getCurrentModuleVariable(), "StandardError", false));

@enebo
Copy link
Member

@enebo enebo commented Jan 19, 2015

I think shortest near term fix would be to change module<0> to Class:Object right? I guess we should figure out it overriding this particular constant will change rescue behavior or not too since manipulating StandardErrror to something else at runtime seems like a horrible idea :) Having an operand for either behavior would probably be the thing to do once we figure out which way it is supposed to work.

BB [6:LBL_2:10]
    %v_3 = recv_ruby_exc
    %v_4 = inheritance_search_const(module<0>)(module<0>, StandardError)
    %v_5 = rescue_eqq(%v_4, %v_3)
    b_true(LBL_5:16, %v_5)

@enebo enebo closed this as completed in eb77f18 Jan 19, 2015
@enebo
Copy link
Member

@enebo enebo commented Jan 19, 2015

Ok so I looked into @subbuss question:

Object.const_set :StandardError, 1

class ExposesJRuby2417 < BasicObject                                               
  def blow_up                                                                      
    begin                                                                          
      ::Kernel.raise "error"                                                       
    rescue 
       puts "rescued"                                                                        
    end                                                                            
  end                                                                              
end                                                                                
ExposesJRuby2417.new.blow_up   

In MRI this prints "rescued" and in JRuby we complain about StandardError being '1'.

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

No branches or pull requests

6 participants