[Truffle] Correct handling of at_exit, exit and abort.

* Do not close stderr when printing a backtrace! * Move #abort to ruby code. * TruffleContextInterface.execute() will execute at_exit handlers since it is fully part of the execution. If a hard exit is not desired, we can pass a parameter or so to just return the exit status. * Properly split running at_exit and actual shutdown. * Fix Thread#kill on the main thread.
jruby · Oct 13, 2015 · ed8e127 · ed8e127
1 parent 1e9936b
commit ed8e127
Show file tree

Hide file tree

Showing 15 changed files with 79 additions and 113 deletions.
diff --git a/core/src/main/java/org/jruby/Main.java b/core/src/main/java/org/jruby/Main.java
@@ -406,8 +406,6 @@ private Status doRunFromMain(Ruby runtime, InputStream in, String filename) {
             doCheckSecurityManager();
 
             runtime.runFromMain(in, filename);
-
-            runtime.shutdownTruffleContextIfRunning();
         } catch (RaiseException rj) {
             return new Status(handleRaiseException(rj));
         }

diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -919,14 +919,6 @@ private TruffleContextInterface loadTruffleContext() {
         return truffleContext;
     }
 
-    public void shutdownTruffleContextIfRunning() {
-        synchronized (truffleContextMonitor) {
-            if (truffleContext != null) {
-                truffleContext.shutdown();
-            }
-        }
-    }
-
     /**
      * @deprecated use #newInstance()
      */

diff --git a/spec/truffle/tags/core/kernel/abort_tags.txt b/spec/truffle/tags/core/kernel/abort_tags.txt
diff --git a/spec/truffle/tags/core/kernel/at_exit_tags.txt b/spec/truffle/tags/core/kernel/at_exit_tags.txt
@@ -2,7 +2,3 @@ slow:Kernel.at_exit runs after all other code
 slow:Kernel.at_exit runs in reverse order of registration
 slow:Kernel.at_exit allows calling exit inside at_exit handler
 slow:Kernel.at_exit gives access to the last raised exception
-fails:Kernel.at_exit gives access to the last raised exception
-fails:Kernel.at_exit runs after all other code
-fails:Kernel.at_exit runs in reverse order of registration
-fails:Kernel.at_exit allows calling exit inside at_exit handler
diff --git a/spec/truffle/tags/core/kernel/raise_tags.txt b/spec/truffle/tags/core/kernel/raise_tags.txt
diff --git a/truffle/src/main/java/org/jruby/truffle/Main.java b/truffle/src/main/java/org/jruby/truffle/Main.java
@@ -74,7 +74,6 @@ public static void main(String[] args) {
                 runtime.getTruffleContext().execute(scriptNode);
             } finally {
                 context.setFileAndLine(oldFile, oldLine);
-                runtime.shutdownTruffleContextIfRunning();
             }
         }
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/KernelNodes.java
@@ -256,32 +256,6 @@ public Object compare(VirtualFrame frame, Object self, Object other) {
 
     }
 
-    @CoreMethod(names = "abort", isModuleFunction = true, optional = 1)
-    public abstract static class AbortNode extends CoreMethodArrayArgumentsNode {
-
-        public AbortNode(RubyContext context, SourceSection sourceSection) {
-            super(context, sourceSection);
-        }
-
-        @TruffleBoundary
-        @Specialization(guards = "isRubyString(message)")
-        public DynamicObject abort(DynamicObject message) {
-            System.err.println(message.toString());
-            return abort();
-        }
-
-        @Specialization
-        public DynamicObject abort(NotProvided message) {
-            return abort();
-        }
-
-        @TruffleBoundary
-        private DynamicObject abort() {
-            getContext().innerShutdown(false);
-            throw new MainExitException(1, true);
-        }
-    }
-
     @CoreMethod(names = "binding", isModuleFunction = true)
     public abstract static class BindingNode extends CoreMethodArrayArgumentsNode {
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/core/ThreadNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/core/ThreadNodes.java
@@ -127,10 +127,15 @@ public static void cleanup(RubyContext context, DynamicObject thread) {
         Layouts.THREAD.getFinishedLatch(thread).countDown();
     }
 
-    public static void shutdown(DynamicObject thread) {
+    public static void shutdown(RubyContext context, DynamicObject thread, Node currentNode) {
         assert RubyGuards.isRubyThread(thread);
         Layouts.THREAD.getFiberManager(thread).shutdown();
-        throw new ThreadExitException();
+
+        if (thread == context.getThreadManager().getRootThread()) {
+            throw new RaiseException(context.getCoreLibrary().systemExit(0, currentNode));
+        } else {
+            throw new ThreadExitException();
+        }
     }
 
     @CoreMethod(names = "alive?")
@@ -179,7 +184,7 @@ public DynamicObject kill(final DynamicObject rubyThread) {
             getContext().getSafepointManager().pauseThreadAndExecuteLater(toKill, this, new SafepointAction() {
                 @Override
                 public void run(DynamicObject currentThread, Node currentNode) {
-                    shutdown(currentThread);
+                    shutdown(getContext(), currentThread, currentNode);
                 }
             });
 

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/exceptions/TopLevelRaiseHandler.java b/truffle/src/main/java/org/jruby/truffle/nodes/exceptions/TopLevelRaiseHandler.java
@@ -10,7 +10,6 @@
 package org.jruby.truffle.nodes.exceptions;
 
 import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.source.SourceSection;
@@ -19,10 +18,9 @@
 import org.jruby.truffle.nodes.cast.IntegerCastNode;
 import org.jruby.truffle.nodes.cast.IntegerCastNodeGen;
 import org.jruby.truffle.runtime.RubyContext;
-import org.jruby.truffle.runtime.backtrace.BacktraceFormatter;
 import org.jruby.truffle.runtime.control.RaiseException;
-import org.jruby.truffle.runtime.control.ThreadExitException;
 import org.jruby.truffle.runtime.layouts.Layouts;
+import org.jruby.truffle.runtime.subsystems.AtExitManager;
 
 public class TopLevelRaiseHandler extends RubyNode {
 
@@ -36,32 +34,35 @@ public TopLevelRaiseHandler(RubyContext context, SourceSection sourceSection, Ru
 
     @Override
     public Object execute(VirtualFrame frame) {
+        DynamicObject lastException = null;
+
         try {
-            return body.execute(frame);
+            body.execute(frame);
         } catch (RaiseException e) {
-            handleException(e);
-        } catch (ThreadExitException e) {
-            // Ignore
+            lastException = AtExitManager.handleAtExitException(getContext(), e);
+        } finally {
+            final DynamicObject atExitException = getContext().runAtExitHooks();
+            if (atExitException != null) {
+                lastException = atExitException;
+            }
+
+            getContext().shutdown();
+            final int status = statusFromException(lastException);
+            System.exit(status);
         }
 
+        // unreachable
         return nil();
     }
 
-    @TruffleBoundary
-    private void handleException(RaiseException e) {
-        final DynamicObject rubyException = e.getRubyException();
-        final int status;
-
-        if (Layouts.BASIC_OBJECT.getLogicalClass(rubyException) == getContext().getCoreLibrary().getSystemExitClass()) {
-            status = castToInt(rubyException.get("@status", null));
+    private int statusFromException(DynamicObject exception) {
+        if (exception == null) {
+            return 0;
+        } else if (Layouts.BASIC_OBJECT.getLogicalClass(exception) == getContext().getCoreLibrary().getSystemExitClass()) {
+            return castToInt(exception.get("@status", null));
         } else {
-            status = 1;
-            BacktraceFormatter.createDefaultFormatter(getContext()).printBacktrace((DynamicObject) rubyException, Layouts.EXCEPTION.getBacktrace((DynamicObject) rubyException));
+            return 1;
         }
-
-        getContext().shutdown();
-
-        System.exit(status);
     }
 
     private int castToInt(Object value) {

diff --git a/truffle/src/main/java/org/jruby/truffle/nodes/rubinius/VMPrimitiveNodes.java b/truffle/src/main/java/org/jruby/truffle/nodes/rubinius/VMPrimitiveNodes.java
@@ -154,8 +154,7 @@ public VMExitPrimitiveNode(RubyContext context, SourceSection sourceSection) {
 
         @Specialization
         public Object vmExit(int status) {
-            getContext().innerShutdown(false);
-
+            getContext().shutdown();
             System.exit(status);
             return nil();
         }

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/RubyContext.java b/truffle/src/main/java/org/jruby/truffle/runtime/RubyContext.java
@@ -435,16 +435,6 @@ public long getNextObjectID() {
         return id;
     }
 
-    public void innerShutdown(boolean normalExit) {
-        atExitManager.run(normalExit);
-
-        if (instrumentationServerManager != null) {
-            instrumentationServerManager.shutdown();
-        }
-
-        threadManager.shutdown();
-    }
-
     public Object makeTuple(VirtualFrame frame, CallDispatchHeadNode newTupleNode, Object... values) {
         return newTupleNode.call(frame, getCoreLibrary().getTupleClass(), "create", null, values);
     }
@@ -670,9 +660,19 @@ public Object execute(final org.jruby.ast.RootNode rootNode) {
         return execute(ParserContext.TOP_LEVEL, DeclarationContext.TOP_LEVEL, newRootNode, null, coreLibrary.getMainObject());
     }
 
+    public DynamicObject runAtExitHooks() {
+        return atExitManager.runAtExitHooks();
+    }
+
     @Override
     public void shutdown() {
-        innerShutdown(true);
+        atExitManager.runSystemExitHooks();
+
+        if (instrumentationServerManager != null) {
+            instrumentationServerManager.shutdown();
+        }
+
+        threadManager.shutdown();
     }
 
     public PrintStream getDebugStandardOut() {

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/backtrace/BacktraceFormatter.java b/truffle/src/main/java/org/jruby/truffle/runtime/backtrace/BacktraceFormatter.java
@@ -68,9 +68,7 @@ public BacktraceFormatter(RubyContext context, EnumSet<FormattingFlags> flags) {
     }
 
     public void printBacktrace(DynamicObject exception, Backtrace backtrace) {
-        try (PrintWriter writer = new PrintWriter(System.err)) {
-            printBacktrace(exception, backtrace, writer);
-        }
+        printBacktrace(exception, backtrace, new PrintWriter(System.err, true));
     }
 
     public void printBacktrace(DynamicObject exception, Backtrace backtrace, PrintWriter writer) {

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java b/truffle/src/main/java/org/jruby/truffle/runtime/core/CoreLibrary.java
@@ -1265,6 +1265,14 @@ public DynamicObject systemCallError(String message, Node currentNode) {
         return ExceptionNodes.createRubyException(systemCallErrorClass, StringOperations.createString(context, StringOperations.encodeByteList(message, UTF8Encoding.INSTANCE)), RubyCallStack.getBacktrace(currentNode));
     }
 
+    public DynamicObject systemExit(int exitStatus, Node currentNode) {
+        CompilerAsserts.neverPartOfCompilation();
+        final DynamicObject message = StringOperations.createString(context, StringOperations.encodeByteList("exit", UTF8Encoding.INSTANCE));
+        final DynamicObject systemExit = ExceptionNodes.createRubyException(systemExitClass, message, RubyCallStack.getBacktrace(currentNode));
+        systemExit.define("@status", exitStatus, 0);
+        return systemExit;
+    }
+
     public RubyContext getContext() {
         return context;
     }

diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/AtExitManager.java b/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/AtExitManager.java
@@ -10,6 +10,7 @@
 package org.jruby.truffle.runtime.subsystems;
 
 import com.oracle.truffle.api.object.DynamicObject;
+
 import org.jruby.truffle.nodes.RubyGuards;
 import org.jruby.truffle.nodes.core.ProcNodes;
 import org.jruby.truffle.runtime.RubyContext;
@@ -27,8 +28,8 @@ public class AtExitManager {
 
     private final RubyContext context;
 
-    private final Deque<DynamicObject> runOnExit = new ConcurrentLinkedDeque<>();
-    private final Deque<DynamicObject> runOnExitAlways = new ConcurrentLinkedDeque<>();
+    private final Deque<DynamicObject> atExitHooks = new ConcurrentLinkedDeque<>();
+    private final Deque<DynamicObject> systemExitHooks = new ConcurrentLinkedDeque<>();
 
     public AtExitManager(RubyContext context) {
         this.context = context;
@@ -38,36 +39,35 @@ public void add(DynamicObject block, boolean always) {
         assert RubyGuards.isRubyProc(block);
 
         if (always) {
-            runOnExitAlways.push(block);
+            systemExitHooks.push(block);
         } else {
-            runOnExit.push(block);
+            atExitHooks.push(block);
         }
     }
 
-    public void run(boolean normalExit) {
-        try {
-            if (normalExit) {
-                runExitHooks(runOnExit);
-            }
-        } finally {
-            runExitHooks(runOnExitAlways);
-        }
+    public DynamicObject runAtExitHooks() {
+        return runExitHooks(atExitHooks);
+    }
+
+    public void runSystemExitHooks() {
+        runExitHooks(systemExitHooks);
     }
 
-    private void runExitHooks(Deque<DynamicObject> stack) {
+    private DynamicObject runExitHooks(Deque<DynamicObject> stack) {
+        DynamicObject lastException = null;
+
         while (true) {
             DynamicObject block;
             try {
                 block = stack.pop();
             } catch (NoSuchElementException e) {
-                break;
+                return lastException;
             }
 
             try {
                 ProcNodes.rootCall(block);
             } catch (RaiseException e) {
-                final Object rubyException = e.getRubyException();
-                BacktraceFormatter.createDefaultFormatter(context).printBacktrace((DynamicObject) rubyException, Layouts.EXCEPTION.getBacktrace((DynamicObject) rubyException));
+                lastException = handleAtExitException(context, e);
             } catch (Exception e) {
                 e.printStackTrace();
             }
@@ -76,9 +76,19 @@ private void runExitHooks(Deque<DynamicObject> stack) {
 
     public List<DynamicObject> getHandlers() {
         final List<DynamicObject> handlers = new ArrayList<>();
-        handlers.addAll(runOnExit);
-        handlers.addAll(runOnExitAlways);
+        handlers.addAll(atExitHooks);
+        handlers.addAll(systemExitHooks);
         return handlers;
     }
 
+    public static DynamicObject handleAtExitException(RubyContext context, RaiseException raiseException) {
+        final DynamicObject rubyException = raiseException.getRubyException();
+        if (Layouts.BASIC_OBJECT.getLogicalClass(rubyException) == context.getCoreLibrary().getSystemExitClass()) {
+            // Do not show SystemExit errors, just track them for the exit status
+        } else {
+            BacktraceFormatter.createDefaultFormatter(context).printBacktrace(rubyException, Layouts.EXCEPTION.getBacktrace(rubyException));
+        }
+        return rubyException;
+    }
+
 }
diff --git a/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/ThreadManager.java b/truffle/src/main/java/org/jruby/truffle/runtime/subsystems/ThreadManager.java
@@ -135,7 +135,7 @@ private void killOtherThreads() {
                     @Override
                     public synchronized void run(DynamicObject thread, Node currentNode) {
                         if (thread != rootThread && Thread.currentThread() == Layouts.THREAD.getThread(thread)) {
-                            ThreadNodes.shutdown(thread);
+                            ThreadNodes.shutdown(context, thread, currentNode);
                         }
                     }
                 });