Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Local vars not restored properly after exception is thrown #6

Closed
ntherning opened this Issue · 2 comments

2 participants

@ntherning

Below is a simple test which demonstrates this issue. The code prints out

  0 = 0
  1 = 1
  2 = 2
  3 = 3

While the expected print out should be:

  0 = 0
  1 = 1
  2 = 2
  3 = 3
public class Exceptions extends Task {
    
    private void f() throws Pausable {
        Task.sleep(10);
        throw new RuntimeException();
    }

    public void execute() throws Pausable{
        int foo = 0;
        Task.sleep(10);
        System.out.println(foo + " = 0");
        foo = 1;
        Task.sleep(10);
        System.out.println(foo + " = 1");
        foo = 2;
        Task.sleep(10);
        System.out.println(foo + " = 2");
        try {
            foo = 3;
            f();
        } catch (Throwable t) {
            System.out.println(foo + " = 3");
        }
    }

    public static void main(String args[]) {
        new Exceptions().start();
    }
}

Here's a patch which solves this problem:

diff --git a/src/kilim/analysis/MethodWeaver.java b/src/kilim/analysis/MethodWeaver.java
index ea63a80..724cce9 100755
--- a/src/kilim/analysis/MethodWeaver.java
+++ b/src/kilim/analysis/MethodWeaver.java
@@ -415,15 +415,16 @@ public class MethodWeaver {
         VMType.loadVar(mv, VMType.TOBJECT, getFiberVar());
         mv.visitMethodInsn(INVOKEVIRTUAL, FIBER_CLASS, "upEx", "()I");
         // fiber.pc is on stack
-        Label[] labels = new Label[cwList.size() + 1];
-        labels[0] = resumeLabel;
+        Label[] labels = new Label[cwList.size()];
+        int[] keys = new int[cwList.size()];
         for (int i = 0; i < cwList.size(); i++) {
-            labels[i + 1] = new Label();
+            labels[i] = new Label();
+            keys[i] = callWeavers.indexOf(cwList.get(i)) + 1;
         }
-        mv.visitTableSwitchInsn(0, cwList.size(), resumeLabel, labels);
-        int i = 1;
+        mv.visitLookupSwitchInsn(resumeLabel, keys, labels);
+        int i = 0;
         for (CallWeaver cw: cwList) {
-            if (i > 1) {
+            if (i > 0) {
                 // This is the jump (to normal exception handling) for the previous
                 // switch case.
                 mv.visitJumpInsn(GOTO, resumeLabel);
@ntherning

The problem occurs when there are pausable calls before the try-catch in the method. upEx() returns the index of the pausable call which threw the exception. The old code in the catch assumed that the first pausable call in the try catch was at pc=1 but if there are pausable calls before the try catch pc will be more than 1. And when the pc was higher than expected the old code would execute the default handler in the tableswitch which meant that local vars were not restored.

@kilim
Owner

Fixed in commit 4e59bbb

Thanks to ntherning for both the simple test case and the fix, and massive apologies for dropping this fix on the floor for so long.

@kilim kilim closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.