Fixes #4865. OOM due to unbounded rescuePCs growth.

The comment in this commit hopefully spells out how the solution works but one tl;dr is we cannot guarantee start and end exception region instructions will occur in the presence of branches or jumps. The workaround is to clean up the stack whenever we decide to add a new exception region. This had a small side-benefit of removing a hack we must have added when seeing a jump-like instr just doing a pop and hoping for the best.
jruby · Nov 27, 2017 · 98d1074 · 98d1074
1 parent 0781932
commit 98d1074
Showing 1 changed file with 34 additions and 7 deletions.
diff --git a/core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java b/core/src/main/java/org/jruby/ir/interpreter/StartupInterpreterEngine.java
@@ -78,9 +78,6 @@ public IRubyObject interpret(ThreadContext context, Block block, IRubyObject sel
                         switch (operation) {
                             case JUMP:
                                 JumpInstr jump = ((JumpInstr)instr);
-                                if (jump.exitsExcRegion()) {
-                                    rescuePCs.pop();
-                                }
                                 ipc = jump.getJumpTarget().getTargetPC();
                                 break;
                             default:
@@ -96,10 +93,26 @@ public IRubyObject interpret(ThreadContext context, Block block, IRubyObject sel
                                 // which will now use the updated value of currDynScope.
                                 currDynScope = interpreterContext.newDynamicScope(context);
                                 context.pushScope(currDynScope);
-                            case EXC_REGION_START:
-                                if (rescuePCs == null) rescuePCs = new Stack<>();
-                                rescuePCs.push(((ExceptionRegionStartMarkerInstr) instr).getFirstRescueBlockLabel().getTargetPC());
-                                break;
+                            case EXC_REGION_START: {
+                                int newPC = ((ExceptionRegionStartMarkerInstr) instr).getFirstRescueBlockLabel().getTargetPC();
+
+                                if (rescuePCs == null) {
+                                    rescuePCs = new Stack<>();
+                                    rescuePCs.push(newPC);
+                                } else {
+                                    // We use EXC_REGION_{START,END} as actual instructions instead of markers
+                                    // in this particular interpreter.  Unfortunately, these can never be guaranteed to
+                                    // execute in matched pairs since other instrs (like from a jump representing a Ruby
+                                    // next) may happen before hitting the END instr.  Because of this we will look to
+                                    // see if the stack is dirty and prune back to a proper clean point.  Otherwise it is
+                                    // clean and we push a new entry.  This mechanism works because these exc. regions
+                                    // represent lexical boundaries and you cannot see the same boundary nested in itself.
+                                    // If we try to push something already there then the space-time continuum is blown
+                                    // and we have to clean the universe up.
+                                    pushOrPrune(newPC, rescuePCs);
+                                }
+                            }
+                            break;
                             case EXC_REGION_END:
                                 rescuePCs.pop();
                                 break;
@@ -137,6 +150,20 @@ public IRubyObject interpret(ThreadContext context, Block block, IRubyObject sel
         throw context.runtime.newRuntimeError("BUG: interpreter fell through to end unexpectedly");
     }
 
+    private void pushOrPrune(int element, Stack<Integer> stack) {
+        int firstOccurrence = stack.indexOf(element);
+
+        if (firstOccurrence != -1) {
+            int size = stack.size();
+
+            for (int i = firstOccurrence + 1; i < size; i++) {
+                stack.remove(firstOccurrence + 1);
+            }
+        } else {
+            stack.push(element);
+        }
+    }
+
     protected static void processOtherOp(ThreadContext context, Block block, Instr instr, Operation operation, DynamicScope currDynScope,
                                          StaticScope currScope, Object[] temp, IRubyObject self) {
         Block.Type blockType = block == null ? null : block.type;