Use a one-object pool and encapculate fields

This allows multiple IRReturnJump to be in flight at once without stepping on each other, and cleans up some direct accesses of the jump fields.
headius · Dec 5, 2023 · 26c5f4a · 26c5f4a
1 parent c4c331b
commit 26c5f4a
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 12 deletions.
diff --git a/core/src/main/java/org/jruby/Ruby.java b/core/src/main/java/org/jruby/Ruby.java
@@ -1302,8 +1302,8 @@ public IRubyObject runInterpreter(ThreadContext context, ParseResult parseResult
              * This will capture any return which says it should return to a script scope as the reasonable
              * exit point.  We still raise when jump off point is anything else since that is a bug.
              */
-            if (!ex.methodToReturnFrom.getStaticScope().getIRScope().isScriptScope()) {
-                System.err.println("Unexpected 'return' escaped the runtime from " + ex.returnScope.getIRScope() + " to " + ex.methodToReturnFrom.getStaticScope().getIRScope());
+            if (!ex.isReturningToScriptScope()) {
+                System.err.println("Unexpected 'return' escaped the runtime: " + ex.toString());
                 System.err.println(ThreadContext.createRawBacktraceStringFromThrowable(ex, false));
                 Throwable t = ex;
                 while ((t = t.getCause()) != null) {

diff --git a/core/src/main/java/org/jruby/ir/runtime/IRReturnJump.java b/core/src/main/java/org/jruby/ir/runtime/IRReturnJump.java
@@ -6,39 +6,58 @@
 import org.jruby.runtime.DynamicScope;
 
 public class IRReturnJump extends IRJump implements Unrescuable {
-    public StaticScope returnScope;
-    public DynamicScope methodToReturnFrom;
-    public Object returnValue;
+    private StaticScope returnScope;
+    private DynamicScope methodToReturnFrom;
+    private Object returnValue;
+    private boolean inUse = false;
 
     private static final ThreadLocal<IRReturnJump> RETURN_JUMP = new ThreadLocal<>();
 
     private IRReturnJump(StaticScope returnScope, DynamicScope scopeToReturnFrom, Object rv) {
-        reset(returnScope, scopeToReturnFrom, rv);
+        update(returnScope, scopeToReturnFrom, rv);
     }
 
     public static IRReturnJump create(StaticScope returnScope, DynamicScope scopeToReturnFrom, Object rv) {
         IRReturnJump jump = RETURN_JUMP.get();
-        if (jump == null) {
+        if (jump == null || jump.inUse) {
             RETURN_JUMP.set(jump = new IRReturnJump(returnScope, scopeToReturnFrom, rv));
         } else {
-            jump.reset(returnScope, scopeToReturnFrom, rv);
+            jump.update(returnScope, scopeToReturnFrom, rv);
         }
 
         return jump;
     }
 
-    private void reset(StaticScope returnScope, DynamicScope scopeToReturnFrom, Object rv) {
+    private void update(StaticScope returnScope, DynamicScope scopeToReturnFrom, Object rv) {
         this.methodToReturnFrom = scopeToReturnFrom;
         this.returnScope = returnScope;
         this.returnValue = rv;
+        this.inUse = true;
+    }
+
+    public void reset() {
+        methodToReturnFrom = null;
+        returnScope = null;
+        returnValue = null;
+        inUse = false;
     }
 
     public boolean isReturnToScope(DynamicScope scope) {
         return methodToReturnFrom == scope;
     }
 
+    public Object returnAndReset() {
+        Object returnValue = this.returnValue;
+        reset();
+        return returnValue;
+    }
+
     @Override
     public String toString() {
-        return "IRReturnJump:<" + methodToReturnFrom + ":" + returnValue + ">";
+        return "IRReturnJump:<" + returnScope.getIRScope() + ":" + methodToReturnFrom.getStaticScope().getIRScope() + ">";
+    }
+
+    public boolean isReturningToScriptScope() {
+        return methodToReturnFrom.getStaticScope().getIRScope().isScriptScope();
     }
 }
diff --git a/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java b/core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
@@ -193,7 +193,7 @@ public static IRubyObject handleNonlocalReturn(DynamicScope currentScope, Object
             // If we are in the method scope we are supposed to return from, stop p<ropagating.
             if (rj.isReturnToScope(currentScope)) {
                 if (isDebug()) System.out.println("---> Non-local Return reached target in scope: " + currentScope);
-                return (IRubyObject) rj.returnValue;
+                return (IRubyObject) rj.returnAndReset();
             }
 
             // If not, Just pass it along!
@@ -243,7 +243,7 @@ public static IRubyObject handleBreakAndReturnsInLambdas(ThreadContext context,
             return ((IRWrappedLambdaReturnValue) exc).returnValue;
         } else if (exc instanceof IRReturnJump && dynScope != null && inReturnToScope(block.type, (IRReturnJump) exc, dynScope)) {
             if (isDebug()) System.out.println("---> Non-local Return reached target in scope: " + dynScope);
-            return (IRubyObject) ((IRReturnJump) exc).returnValue;
+            return (IRubyObject) ((IRReturnJump) exc).returnAndReset();
         } else {
             // Propagate the exception
             context.setSavedExceptionInLambda((Throwable) exc);