Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: JRuby 1.5
    • Fix Version/s: None
    • Component/s: Interpreter
    • Labels:
      None
    • Number of attachments :
      1

      Description

      The "rescue" operator fails to catch subclasses of Errno::ENOENT, even when they're explicitly specified. This behavior does not match that of MRI. See attached file. Here's a comparison of this script on MRI vs. JRuby:

      tony@virtualmac:~$ ruby enoent_bug.rb # On MRI
      Everything turned out great!
      tony@virtualmac:~$ rvm use jruby
      Using jruby 1.5.0
      tony@virtualmac:~$ ruby enoent_bug.rb
      enoent_bug.rb:4: No such file or directory - baz (Errno::ENOENT)

        Activity

        Hide
        Charles Oliver Nutter added a comment -

        Appears to be fixed.

        Show
        Charles Oliver Nutter added a comment - Appears to be fixed.
        Hide
        Hiro Asari added a comment -

        I haven't looked into it too deep, but this comment may pique someone's interest.

        Charlie's patch triggers 15 failures and 298 errors against the master. A lot of the errors (maybe all--I just skimmed) are this:

             [java] 200)
             [java] An exception occurred during: loading /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb ERROR
             [java] TypeError: can't convert Module into Integer
             [java] /Users/asari/Development/src/jruby/lib/ruby/1.8/delegate.rb:280:in `DelegateClass'
             [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/fixtures/classes.rb:62
             [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/fixtures/classes.rb:2:in `require'
             [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb:2
             [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb:56:in `load'
             [java] 
        

        while failures are:

             [java] 24)
             [java] SystemCallError.new constructs the appropriate Errno class FAILED
             [java] Expected #<SystemCallError: #<SystemCallError:0x3c8fa832>> (SystemCallError)
             [java] to be an instance of Errno::EINVAL
             [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:22
             [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:3
             [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:56:in `load'
             [java] 
        
        Show
        Hiro Asari added a comment - I haven't looked into it too deep, but this comment may pique someone's interest. Charlie's patch triggers 15 failures and 298 errors against the master. A lot of the errors (maybe all--I just skimmed) are this: [java] 200) [java] An exception occurred during: loading /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb ERROR [java] TypeError: can't convert Module into Integer [java] /Users/asari/Development/src/jruby/lib/ruby/1.8/delegate.rb:280:in `DelegateClass' [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/fixtures/classes.rb:62 [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/fixtures/classes.rb:2:in `require' [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb:2 [java] /Users/asari/Development/src/jruby/spec/ruby/library/delegate/delegate_class/instance_methods_spec.rb:56:in `load' [java] while failures are: [java] 24) [java] SystemCallError.new constructs the appropriate Errno class FAILED [java] Expected #<SystemCallError: #<SystemCallError:0x3c8fa832>> (SystemCallError) [java] to be an instance of Errno::EINVAL [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:22 [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:3 [java] /Users/asari/Development/src/jruby/spec/ruby/core/exception/system_call_error_spec.rb:56:in `load' [java]
        Hide
        Charles Oliver Nutter added a comment - - edited

        Looks like this is caused by commit 10e567b3 which fixed an issue where SystemCallError.new is supposed to return instance of Errno subclasses under certain circumstances. However it appears that this logic, placed into SystemCallError#initialize, should have been in SystemCallError.new.

        Here's a patch that seems to make the case pass, but I have not run tests and have not checked whether it changes other behaviors in the process:

        diff --git a/src/org/jruby/RubySystemCallError.java b/src/org/jruby/RubySystemCallError.java
        index 52868c1..fbaa9a5 100644
        --- a/src/org/jruby/RubySystemCallError.java
        +++ b/src/org/jruby/RubySystemCallError.java
        @@ -20,6 +20,7 @@ import org.jruby.runtime.marshal.MarshalStream;
         import org.jruby.runtime.marshal.UnmarshalStream;
         
         import com.kenai.constantine.platform.Errno;
        +import org.jruby.runtime.ThreadContext;
         
         @JRubyClass(name="SystemCallError", parent="StandardError")
         public class RubySystemCallError extends RubyException {
        @@ -174,79 +175,84 @@ public class RubySystemCallError extends RubyException {
                 
                 runtime.callbackFactory(RubyClass.class);
                 exceptionClass.defineAnnotatedMethods(RubySystemCallError.class);
        +        exceptionClass.getSingletonClass().defineAnnotatedMethods(SystemCallErrorNew.class);
         
                 return exceptionClass;
             }
        -    
        -    @JRubyMethod(optional = 2, required=0, frame = true, visibility = Visibility.PRIVATE)
        -    public IRubyObject initialize(IRubyObject[] args, Block block) {
        -        Ruby runtime = getRuntime();
        -        RubyClass sCallErorrClass = runtime.getSystemCallError();
        -        RubyClass klass = getMetaClass().getRealClass();
         
        -        IRubyObject msg = runtime.getNil();
        -        IRubyObject err = runtime.getNil();
        +    public static class SystemCallErrorNew {
        +        @JRubyMethod(name = "new", optional = 2, required=0, frame = true, meta = true)
        +        public static IRubyObject newInstance(ThreadContext context, IRubyObject self, IRubyObject[] args, Block block) {
        +            Ruby runtime = context.getRuntime();
        +            RubyClass sCallErorrClass = runtime.getSystemCallError();
        +            RubyClass klass = (RubyClass)self;
         
        -        boolean isErrnoClass = !klass.equals(sCallErorrClass);
        +            IRubyObject msg = runtime.getNil();
        +            IRubyObject err = runtime.getNil();
         
        -        if (!isErrnoClass) {
        -            // one optional, one required args
        -            Arity.checkArgumentCount(runtime, args, 1, 2);
        -            msg = args[0];
        -            if (args.length == 2) {
        -                err = args[1];
        -            }
        -            if (args.length == 1 && (msg instanceof RubyFixnum)) {
        -                err = msg;
        -                msg = runtime.getNil();
        -            }
        -        } else {
        -            // one optional and no required args
        -            Arity.checkArgumentCount(runtime, args, 0, 1);
        -            if (args.length == 1) {
        +            boolean isErrnoClass = !klass.equals(sCallErorrClass);
        +
        +            if (!isErrnoClass) {
        +                // one optional, one required args
        +                Arity.checkArgumentCount(runtime, args, 1, 2);
                         msg = args[0];
        +                if (args.length == 2) {
        +                    err = args[1];
        +                }
        +                if (args.length == 1 && (msg instanceof RubyFixnum)) {
        +                    err = msg;
        +                    msg = runtime.getNil();
        +                }
        +            } else {
        +                // one optional and no required args
        +                Arity.checkArgumentCount(runtime, args, 0, 1);
        +                if (args.length == 1) {
        +                    msg = args[0];
        +                }
        +                // try to get errno value out of the class
        +                err = klass.fastGetConstant("Errno");
                     }
        -            // try to get errno value out of the class
        -            err = klass.fastGetConstant("Errno");
        -        }
         
        -        String val = null;
        +            String val = null;
        +
        +            RubySystemCallError sysCallError = new RubySystemCallError(runtime, klass);
         
        -        if (!err.isNil()) {
        -            errno = err.convertToInteger();
        -            int errnoVal = RubyNumeric.num2int(errno);
        -            if (Errno.valueOf(errnoVal) != Errno.__UNKNOWN_CONSTANT__) {
        -                // we got a valid errno value
        -                isErrnoClass = true;
        -                setMetaClass(runtime.getErrno(errnoVal));
        -                klass = getMetaClass().getRealClass();
        +            if (!err.isNil()) {
        +                sysCallError.errno = err.convertToInteger();
        +                int errnoVal = RubyNumeric.num2int(sysCallError.errno);
        +                if (Errno.valueOf(errnoVal) != Errno.__UNKNOWN_CONSTANT__) {
        +                    // we got a valid errno value
        +                    isErrnoClass = true;
        +                    sysCallError.setMetaClass(runtime.getErrno(errnoVal));
        +                    klass = sysCallError.getMetaClass().getRealClass();
         
        -                // FIXME: Errno descriptions from Constantine
        -                // on Windows are not useful at the moment.
        -                if (!Platform.IS_WINDOWS) {
        -                  val = Errno.valueOf(errnoVal).description();
        +                    // FIXME: Errno descriptions from Constantine
        +                    // on Windows are not useful at the moment.
        +                    if (!Platform.IS_WINDOWS) {
        +                      val = Errno.valueOf(errnoVal).description();
        +                    }
                         }
                     }
        -        }
         
        -        if (val == null) {
        -            val = defaultMessages.get(klass.getName());
                     if (val == null) {
        -                val = "Unknown error";
        +                val = defaultMessages.get(klass.getName());
        +                if (val == null) {
        +                    val = "Unknown error";
        +                }
                     }
        -        }
         
        -        // MRI behavior: we don't print errno for actual Errno errors
        -        if (!errno.isNil() && !isErrnoClass) {
        -            val += " " + errno.toString();
        -        }
        +            // MRI behavior: we don't print errno for actual Errno errors
        +            if (!sysCallError.errno.isNil() && !isErrnoClass) {
        +                val += " " + sysCallError.errno.toString();
        +            }
         
        -        if (!msg.isNil()) {
        -            val += " - " + msg.convertToString();
        -        }
        +            if (!msg.isNil()) {
        +                val += " - " + msg.convertToString();
        +            }
         
        -        message = runtime.newString(val);
        -        return this;
        +            sysCallError.message = runtime.newString(val);
        +            return sysCallError;
        +        }
             }
         
             @JRubyMethod
        
        Show
        Charles Oliver Nutter added a comment - - edited Looks like this is caused by commit 10e567b3 which fixed an issue where SystemCallError.new is supposed to return instance of Errno subclasses under certain circumstances. However it appears that this logic, placed into SystemCallError#initialize, should have been in SystemCallError.new. Here's a patch that seems to make the case pass, but I have not run tests and have not checked whether it changes other behaviors in the process: diff --git a/src/org/jruby/RubySystemCallError.java b/src/org/jruby/RubySystemCallError.java index 52868c1..fbaa9a5 100644 --- a/src/org/jruby/RubySystemCallError.java +++ b/src/org/jruby/RubySystemCallError.java @@ -20,6 +20,7 @@ import org.jruby.runtime.marshal.MarshalStream; import org.jruby.runtime.marshal.UnmarshalStream; import com.kenai.constantine.platform.Errno; +import org.jruby.runtime.ThreadContext; @JRubyClass(name="SystemCallError", parent="StandardError") public class RubySystemCallError extends RubyException { @@ -174,79 +175,84 @@ public class RubySystemCallError extends RubyException { runtime.callbackFactory(RubyClass.class); exceptionClass.defineAnnotatedMethods(RubySystemCallError.class); + exceptionClass.getSingletonClass().defineAnnotatedMethods(SystemCallErrorNew.class); return exceptionClass; } - - @JRubyMethod(optional = 2, required=0, frame = true, visibility = Visibility.PRIVATE) - public IRubyObject initialize(IRubyObject[] args, Block block) { - Ruby runtime = getRuntime(); - RubyClass sCallErorrClass = runtime.getSystemCallError(); - RubyClass klass = getMetaClass().getRealClass(); - IRubyObject msg = runtime.getNil(); - IRubyObject err = runtime.getNil(); + public static class SystemCallErrorNew { + @JRubyMethod(name = "new", optional = 2, required=0, frame = true, meta = true) + public static IRubyObject newInstance(ThreadContext context, IRubyObject self, IRubyObject[] args, Block block) { + Ruby runtime = context.getRuntime(); + RubyClass sCallErorrClass = runtime.getSystemCallError(); + RubyClass klass = (RubyClass)self; - boolean isErrnoClass = !klass.equals(sCallErorrClass); + IRubyObject msg = runtime.getNil(); + IRubyObject err = runtime.getNil(); - if (!isErrnoClass) { - // one optional, one required args - Arity.checkArgumentCount(runtime, args, 1, 2); - msg = args[0]; - if (args.length == 2) { - err = args[1]; - } - if (args.length == 1 && (msg instanceof RubyFixnum)) { - err = msg; - msg = runtime.getNil(); - } - } else { - // one optional and no required args - Arity.checkArgumentCount(runtime, args, 0, 1); - if (args.length == 1) { + boolean isErrnoClass = !klass.equals(sCallErorrClass); + + if (!isErrnoClass) { + // one optional, one required args + Arity.checkArgumentCount(runtime, args, 1, 2); msg = args[0]; + if (args.length == 2) { + err = args[1]; + } + if (args.length == 1 && (msg instanceof RubyFixnum)) { + err = msg; + msg = runtime.getNil(); + } + } else { + // one optional and no required args + Arity.checkArgumentCount(runtime, args, 0, 1); + if (args.length == 1) { + msg = args[0]; + } + // try to get errno value out of the class + err = klass.fastGetConstant("Errno"); } - // try to get errno value out of the class - err = klass.fastGetConstant("Errno"); - } - String val = null; + String val = null; + + RubySystemCallError sysCallError = new RubySystemCallError(runtime, klass); - if (!err.isNil()) { - errno = err.convertToInteger(); - int errnoVal = RubyNumeric.num2int(errno); - if (Errno.valueOf(errnoVal) != Errno.__UNKNOWN_CONSTANT__) { - // we got a valid errno value - isErrnoClass = true; - setMetaClass(runtime.getErrno(errnoVal)); - klass = getMetaClass().getRealClass(); + if (!err.isNil()) { + sysCallError.errno = err.convertToInteger(); + int errnoVal = RubyNumeric.num2int(sysCallError.errno); + if (Errno.valueOf(errnoVal) != Errno.__UNKNOWN_CONSTANT__) { + // we got a valid errno value + isErrnoClass = true; + sysCallError.setMetaClass(runtime.getErrno(errnoVal)); + klass = sysCallError.getMetaClass().getRealClass(); - // FIXME: Errno descriptions from Constantine - // on Windows are not useful at the moment. - if (!Platform.IS_WINDOWS) { - val = Errno.valueOf(errnoVal).description(); + // FIXME: Errno descriptions from Constantine + // on Windows are not useful at the moment. + if (!Platform.IS_WINDOWS) { + val = Errno.valueOf(errnoVal).description(); + } } } - } - if (val == null) { - val = defaultMessages.get(klass.getName()); if (val == null) { - val = "Unknown error"; + val = defaultMessages.get(klass.getName()); + if (val == null) { + val = "Unknown error"; + } } - } - // MRI behavior: we don't print errno for actual Errno errors - if (!errno.isNil() && !isErrnoClass) { - val += " " + errno.toString(); - } + // MRI behavior: we don't print errno for actual Errno errors + if (!sysCallError.errno.isNil() && !isErrnoClass) { + val += " " + sysCallError.errno.toString(); + } - if (!msg.isNil()) { - val += " - " + msg.convertToString(); - } + if (!msg.isNil()) { + val += " - " + msg.convertToString(); + } - message = runtime.newString(val); - return this; + sysCallError.message = runtime.newString(val); + return sysCallError; + } } @JRubyMethod

          People

          • Assignee:
            Charles Oliver Nutter
            Reporter:
            Tony Arcieri
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: