Some more $! save/restore fixes + add comments for all of them

jruby · Feb 14, 2015 · b27868a · b27868a
1 parent 78e968e
commit b27868a
Showing 16 changed files with 40 additions and 33 deletions.
diff --git a/core/src/main/java/org/jruby/Main.java b/core/src/main/java/org/jruby/Main.java
@@ -438,18 +438,18 @@ private boolean checkFileSyntax(Ruby runtime, String filename) {
     }
 
     private boolean checkStreamSyntax(Ruby runtime, InputStream in, String filename) {
-        IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+        IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             runtime.parseFromMain(in, filename);
             config.getOutput().println("Syntax OK");
             return true;
         } catch (RaiseException re) {
             if (re.getException().getMetaClass().getBaseName().equals("SyntaxError")) {
-                runtime.getGlobalVariables().set("$!", oldExc);
                 config.getError().println("SyntaxError in " + re.getException().message(runtime.getCurrentContext()));
             } else {
                 throw re;
             }
+            runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             return false;
         }
     }

diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -3228,7 +3228,7 @@ public void tearDown(boolean systemExit) {
 
         while (!atExitBlocks.empty()) {
             RubyProc proc = atExitBlocks.pop();
-            IRubyObject oldExc = context.runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = context.runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 proc.call(getCurrentContext(), IRubyObject.NULL_ARRAY);
             } catch (RaiseException rj) {

diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -684,13 +684,13 @@ private static IRubyObject checkFuncallMissing(ThreadContext context, RubyClass
             return null;
         }
         else {
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 return checkFuncallExec(context, self, method, args);
             } catch (RaiseException e) {
-                // restore $!
-                runtime.getGlobalVariables().set("$!", oldExc);
-                return checkFuncallFailed(context, self, method, runtime.getNoMethodError(), args);
+                IRubyObject ret = checkFuncallFailed(context, self, method, runtime.getNoMethodError(), args);
+                runtime.getGlobalVariables().set("$!", oldExc); // restore $!
+                return ret;
             }
         }
     }

diff --git a/core/src/main/java/org/jruby/RubyEnumerator.java b/core/src/main/java/org/jruby/RubyEnumerator.java
@@ -756,7 +756,7 @@ public void run() {
             IRubyObject finalObject = NEVER;
 
             try {
-                IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+                IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
                 final TerminateEnumeration terminateEnumeration = new TerminateEnumeration();
                 try {
                     object.callMethod(context, method, methodArgs, CallBlock.newCallClosure(object, object.getMetaClass(), Arity.OPTIONAL, new BlockCallback() {
@@ -784,9 +784,9 @@ public IRubyObject call(ThreadContext context, IRubyObject[] args, Block block)
                     }
                     // ignore, we're shutting down
                 } catch (RaiseException re) {
-                    runtime.getGlobalVariables().set("$!", oldExc);
                     if (DEBUG) System.out.println(Thread.currentThread().getName() + ": exception at toplevel: " + re.getException());
                     finalObject = re.getException();
+                    runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
                 } catch (Throwable t) {
                     if (DEBUG) {
                         System.out.println(Thread.currentThread().getName() + ": exception at toplevel: " + t);

diff --git a/core/src/main/java/org/jruby/RubyFileTest.java b/core/src/main/java/org/jruby/RubyFileTest.java
@@ -277,7 +277,7 @@ public static IRubyObject sticky_p(IRubyObject recv, IRubyObject filename) {
     @JRubyMethod(name = "symlink?", required = 1, module = true)
     public static RubyBoolean symlink_p(IRubyObject recv, IRubyObject filename) {
         Ruby runtime = recv.getRuntime();
-        IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+        IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
 
         try {
             // Note: We can't use file.exists() to check whether the symlink
@@ -291,7 +291,7 @@ public static RubyBoolean symlink_p(IRubyObject recv, IRubyObject filename) {
         } catch (SecurityException re) {
             return runtime.getFalse();
         } catch (RaiseException re) {
-            runtime.getGlobalVariables().set("$!", oldExc);
+            runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             return runtime.getFalse();
         }
     }

diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java
@@ -1942,13 +1942,13 @@ protected static IRubyObject ioClose(Ruby runtime, IRubyObject io) {
         ThreadContext context = runtime.getCurrentContext();
         IRubyObject closed = io.checkCallMethod(context, "closed?");
         if (closed != null && closed.isTrue()) return io;
-        IRubyObject oldExc = context.getErrorInfo();
+        IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             return io.checkCallMethod(context, "close");
         } catch (RaiseException re) {
             if (re.getMessage().contains(CLOSED_STREAM_MSG)) {
                 // ignore
-                context.setErrorInfo(oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
                 return context.nil;
             } else {
                 throw re;

diff --git a/core/src/main/java/org/jruby/RubyKernel.java b/core/src/main/java/org/jruby/RubyKernel.java
@@ -1309,7 +1309,7 @@ public static IRubyObject loop(ThreadContext context, IRubyObject recv, Block bl
         }
         IRubyObject nil = runtime.getNil();
         RubyClass stopIteration = runtime.getStopIteration();
-        IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+        IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             while (true) {
                 block.yieldSpecific(context);
@@ -1320,7 +1320,7 @@ public static IRubyObject loop(ThreadContext context, IRubyObject recv, Block bl
             if (!stopIteration.op_eqq(context, ex.getException()).isTrue()) {
                 throw ex;
             } else {
-                runtime.getGlobalVariables().set("$!", oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             }
         }
         return nil;
@@ -1678,7 +1678,7 @@ private static IRubyObject execCommon(Ruby runtime, IRubyObject env, IRubyObject
         System.setProperty("user.dir", runtime.getCurrentDirectory());
 
         if (nativeExec) {
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 ShellLauncher.LaunchConfig cfg = new ShellLauncher.LaunchConfig(runtime, args, true);
 
@@ -1716,7 +1716,7 @@ private static IRubyObject execCommon(Ruby runtime, IRubyObject env, IRubyObject
                 // Only here because native exec could not exec (always -1)
                 nativeFailed = true;
             } catch (RaiseException e) {
-                runtime.getGlobalVariables().set("$!", oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             } catch (Exception e) {
                 throw runtime.newErrnoENOENTError("cannot execute: " + e.getLocalizedMessage());
             }

diff --git a/core/src/main/java/org/jruby/RubyModule.java b/core/src/main/java/org/jruby/RubyModule.java
@@ -859,15 +859,14 @@ public void undef(ThreadContext context, String name) {
         }
 
         if (name.equals("method_missing")) {
-
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 removeMethod(context, name);
             } catch (RaiseException t) {
                 if (!(t.getException() instanceof RubyNameError)) {
                     throw t;
                 } else {
-                    runtime.getGlobalVariables().set("$!", oldExc);
+                    runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
                 }
             }
             return;

diff --git a/core/src/main/java/org/jruby/RubyNumeric.java b/core/src/main/java/org/jruby/RubyNumeric.java
@@ -409,15 +409,15 @@ public double yield(RubyString arg, boolean strict) {
     protected IRubyObject[] getCoerced(ThreadContext context, IRubyObject other, boolean error) {
         IRubyObject result;
 
-        IRubyObject savedError = context.runtime.getGlobalVariables().get("$!");
+        IRubyObject savedError = context.runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             result = other.callMethod(context, "coerce", this);
         } catch (RaiseException e) {
             if (error) {
                 throw getRuntime().newTypeError(
                         other.getMetaClass().getName() + " can't be coerced into " + getMetaClass().getName());
             } else {
-                context.runtime.getGlobalVariables().set("$!", savedError);
+                context.runtime.getGlobalVariables().set("$!", savedError); // Restore $!
             }
 
             return null;
@@ -461,7 +461,7 @@ protected final IRubyObject coerceBody(ThreadContext context, IRubyObject other)
     protected final RubyArray doCoerce(ThreadContext context, IRubyObject other, boolean err) {
         IRubyObject result;
 
-        IRubyObject savedError = context.runtime.getGlobalVariables().get("$!");
+        IRubyObject savedError = context.runtime.getGlobalVariables().get("$!"); // Svae $!
         try {
             result = coerceBody(context, other);
         } catch (RaiseException e) {
@@ -471,8 +471,7 @@ protected final RubyArray doCoerce(ThreadContext context, IRubyObject other, boo
             if (err) {
                 coerceFailed(context, other);
             }
-            // Restore $!
-            context.runtime.getGlobalVariables().set("$!", savedError);
+            context.runtime.getGlobalVariables().set("$!", savedError); // Restore $!
             return null;
         }
 

diff --git a/core/src/main/java/org/jruby/RubyTime.java b/core/src/main/java/org/jruby/RubyTime.java
@@ -1216,21 +1216,21 @@ protected static RubyTime s_mload(IRubyObject recv, RubyTime time, IRubyObject f
 
         int offset = 0;
         if (offsetVar != null && offsetVar.respondsTo("to_int")) {
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 offset = offsetVar.convertToInteger().getIntValue() * 1000;
             } catch (RaiseException typeError) {
-                runtime.getGlobalVariables().set("$!", oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             }
         }
 
         String zone = "";
         if (zoneVar != null && zoneVar.respondsTo("to_str")) {
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 zone = zoneVar.convertToString().toString();
             } catch (RaiseException typeError) {
-                runtime.getGlobalVariables().set("$!", oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             }
         }
 

diff --git a/core/src/main/java/org/jruby/ext/etc/RubyEtc.java b/core/src/main/java/org/jruby/ext/etc/RubyEtc.java
@@ -116,6 +116,7 @@ private static IRubyObject intoStringArray(Ruby runtime, String[] members) {
     public static IRubyObject getpwuid(IRubyObject recv, IRubyObject[] args) {
         Ruby runtime = recv.getRuntime();
         POSIX posix = runtime.getPosix();
+        IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             int uid = args.length == 0 ? posix.getuid() : RubyNumeric.fix2int(args[0]);
             Passwd pwd = posix.getpwuid(uid);
@@ -128,6 +129,7 @@ public static IRubyObject getpwuid(IRubyObject recv, IRubyObject[] args) {
             return setupPasswd(runtime, pwd);
         } catch (RaiseException re) {
             if (runtime.getNotImplementedError().isInstance(re.getException())) {
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
                 return runtime.getNil();
             }
             throw re;

diff --git a/core/src/main/java/org/jruby/ext/pathname/RubyPathname.java b/core/src/main/java/org/jruby/ext/pathname/RubyPathname.java
@@ -394,12 +394,14 @@ public IRubyObject each_entry(ThreadContext context, Block block) {
 
     @JRubyMethod(name = {"unlink", "delete"})
     public IRubyObject unlink(ThreadContext context) {
+        IRubyObject oldExc = context.runtime.getGlobalVariables().get("$!"); // Save $!
         try {
             return context.runtime.getDir().callMethod(context, "unlink", getPath());
         } catch (RaiseException ex) {
             if (!context.runtime.getErrno().getClass("ENOTDIR").isInstance(ex.getException())) {
                 throw ex;
             }
+            context.runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             return context.runtime.getFile().callMethod(context, "unlink", getPath());
         }
     }

diff --git a/core/src/main/java/org/jruby/ext/tempfile/Tempfile.java b/core/src/main/java/org/jruby/ext/tempfile/Tempfile.java
@@ -203,17 +203,18 @@ public IRubyObject unlink(ThreadContext context) {
         POSIX posix = runtime.getPosix();
 
         if (posix.isNative() && !Platform.IS_WINDOWS) {
+            IRubyObject oldExc = context.runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 RubyFile.unlink(context, this);
             } catch (RaiseException re) {
                 RubyException excp = re.getException();
                 if (!(excp instanceof RubySystemCallError)) throw re;
 
                 int errno = (int)((RubySystemCallError)excp).errno().convertToInteger().getLongValue();
-                if (errno != Errno.ENOENT.intValue() &&
-                        errno != Errno.EACCES.intValue()) {
+                if (errno != Errno.ENOENT.intValue() && errno != Errno.EACCES.intValue()) {
                     throw re;
                 }
+                context.runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             }
             openFile.setPath(null);
             tmpname = context.nil;

diff --git a/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java b/core/src/main/java/org/jruby/java/proxies/MapJavaProxy.java
@@ -86,10 +86,12 @@ private RubyHashMap getOrCreateRubyHashMap() {
         }
         // (JavaProxy)recv).getObject() might raise exception when
         // wrong number of args are given to the constructor.
+        IRubyObject oldExc = getRuntime().getGlobalVariables().get("$!"); // Save $!
         try {
             wrappedMap.setSize(((Map)((JavaProxy)this).getObject()).size());
         } catch (RaiseException e) {
             wrappedMap.setSize(0);
+            getRuntime().getGlobalVariables().set("$!", oldExc); // Restore $!
         }
         return wrappedMap;
     }

diff --git a/core/src/main/java/org/jruby/management/Runtime.java b/core/src/main/java/org/jruby/management/Runtime.java
@@ -109,10 +109,12 @@ public String executeRuby(final String code) {
 
             @Override
             public void run() {
+                // IRubyObject oldExc = ruby.get().getGlobalVariables().get("$!"); // Save $!
                 try {
                     result[0] = ruby.get().evalScriptlet(code).toString();
                 } catch (RaiseException re) {
                     result[0] = ruby.get().getInstanceConfig().getTraceType().printBacktrace(re.getException(), false);
+                    // ruby.get().getGlobalVariables().set("$!", oldExc); // Restore $!
                 } catch (Throwable t) {
                     StringWriter sw = new StringWriter();
                     t.printStackTrace(new PrintWriter(sw));

diff --git a/core/src/main/java/org/jruby/util/SunSignalFacade.java b/core/src/main/java/org/jruby/util/SunSignalFacade.java
@@ -81,7 +81,7 @@ private JRubySignalHandler(Ruby runtime, IRubyObject block, BlockCallback callba
 
         public void handle(Signal signal) {
             ThreadContext context = runtime.getCurrentContext();
-            IRubyObject oldExc = runtime.getGlobalVariables().get("$!");
+            IRubyObject oldExc = runtime.getGlobalVariables().get("$!"); // Save $!
             try {
                 if (block != null) {
                     block.callMethod(context, "call");
@@ -93,7 +93,7 @@ public void handle(Signal signal) {
                     runtime.getThread().callMethod(context, "main")
                         .callMethod(context, "raise", e.getException());
                 } catch(Exception ignored) {}
-                runtime.getGlobalVariables().set("$!", oldExc);
+                runtime.getGlobalVariables().set("$!", oldExc); // Restore $!
             } catch (MainExitException mee) {
                 runtime.getThreadService().getMainThread().kill();
             } finally {