commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kohs...@apache.org
Subject svn commit: r307265 - in /jakarta/commons/sandbox/javaflow/trunk/src: java/org/apache/commons/javaflow/bytecode/ test/org/apache/commons/javaflow/ test/org/apache/commons/javaflow/testcode/
Date Sat, 08 Oct 2005 05:48:06 GMT
Author: kohsuke
Date: Fri Oct  7 22:47:58 2005
New Revision: 307265

URL: http://svn.apache.org/viewcvs?rev=307265&view=rev
Log:
fixed a bug in the stack restoration.

the way the reference stack works is that when a stack frame is saved,
the callee pushs to the rstack, then when restored, the caller pops
from the rstack. So when the top-level Runnable object returns, the top
of the rstack if that Runnable object. Previously we were using this to
fetch the first Runnable, so everything was OK. But now that we use a
separate 'runnable' field, this top rstack object needs to be removed
or else the restoration won't work correctly.

Also note that the same Runnable object is normally kept in the ostack
as well, as most of the time local variable #0 is used for the 'this'
variable.

IMO, this makes it somewhat useless to introduce the 'runnable' variable,
as you won't be able to change it and expect that the stack restores
correctly --- the only thing you'll be able to do is to inspect the value,
which could be just as well done by defining the 'getRunnable' method
that checks for the top of the rstack.

In any case, for now I just fixed the problem and left the design
as-is. I also added a test case to prevent future regressions.

Added:
    jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/testcode/BlackRed.java
Modified:
    jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java
    jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
    jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java

Modified: jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java?rev=307265&r1=307264&r2=307265&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java
(original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/java/org/apache/commons/javaflow/bytecode/StackRecorder.java
Fri Oct  7 22:47:58 2005
@@ -84,33 +84,40 @@
     public StackRecorder execute(final Object context) {
         final StackRecorder old = registerThread();
         try {
-            Object target = runnable;
-            
             restoring = !isEmpty(); // start restoring if we have a filled stack
             this.context = context;
             
             if (restoring) {
-                log.debug("Restoring state of " + ReflectionUtils.getClassName(target) +
"/" + ReflectionUtils.getClassLoaderName(target));
-            }
-            
-            if (target instanceof Runnable) {
-                log.debug("calling runnable");
-                ((Runnable)target).run();                
-            } else {
-                log.error("No runnable on stack. " + ReflectionUtils.getClassName(target)
+ "/" + ReflectionUtils.getClassLoaderName(target));
+                log.debug("Restoring state of " + ReflectionUtils.getClassName(runnable)
+ "/" + ReflectionUtils.getClassLoaderName(runnable));
             }
             
+            log.debug("calling runnable");
+            runnable.run();
+
             if (capturing) {
+                // the top of the reference stack is always the same as 'runnable'.
+                // since we won't use this (instead we use 'runnable') for restoring
+                // the stack frame, we need to throw it away now, or else the restoration
won't work.
+                if(isEmpty() || popReference()!=runnable) {
+                    // if we were really capturing the stack, at least we should have
+                    // one object in the reference stack. Otherwise, it usually means
+                    // that the application wasn't instrumented correctly.
+                    throw new IllegalStateException("stack corruption. Is "+runnable.getClass()+"
instrumented for javaflow?");
+                }
                 return this;
+            } else {
+                return null;    // nothing more to continue
             }
-        } catch(Throwable t) {
-            log.error(t.getMessage(),t);
+        } catch(Error e) {
+            log.error(e.getMessage(),e);
+            throw e;
+        } catch(RuntimeException e) {
+            log.error(e.getMessage(),e);
+            throw e;
         } finally {
             this.context = null;
             deregisterThread(old);
         }
-        
-        return null;
     }
 
     public Object getContext() {

Modified: jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java?rev=307265&r1=307264&r2=307265&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
(original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTestCase.java
Fri Oct  7 22:47:58 2005
@@ -46,4 +46,8 @@
     public void testStackBug() throws Exception {
         call("testStackBug");
     }
+
+    public void testBlackRed() throws Exception {
+        call("testBlackRed");
+    }
 }

Modified: jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java?rev=307265&r1=307264&r2=307265&view=diff
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java
(original)
+++ jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/ContinuationTests.java
Fri Oct  7 22:47:58 2005
@@ -20,6 +20,7 @@
 import org.apache.commons.javaflow.testcode.Counter;
 import org.apache.commons.javaflow.testcode.NewObject;
 import org.apache.commons.javaflow.testcode.StackBug;
+import org.apache.commons.javaflow.testcode.BlackRed;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -95,6 +96,13 @@
 
     public void testStackBug() throws Exception {
         Continuation c = Continuation.startWith(new StackBug());
+        assertNull(c);
+    }
+
+    public void testBlackRed() throws Exception {
+        Continuation c = Continuation.startWith(new BlackRed());
+        assertNotNull(c);
+        c = Continuation.continueWith(c);
         assertNull(c);
     }
 }

Added: jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/testcode/BlackRed.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/testcode/BlackRed.java?rev=307265&view=auto
==============================================================================
--- jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/testcode/BlackRed.java
(added)
+++ jakarta/commons/sandbox/javaflow/trunk/src/test/org/apache/commons/javaflow/testcode/BlackRed.java
Fri Oct  7 22:47:58 2005
@@ -0,0 +1,54 @@
+package org.apache.commons.javaflow.testcode;
+
+import org.apache.commons.javaflow.Continuation;
+
+import java.io.Serializable;
+
+/**
+ * Test for making sure that rstack works correctly.
+ *
+ * For this test we need to have a stack frame that goes through multiple objects
+ * of different types.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+public class BlackRed implements Runnable, Serializable {
+    public void run() {
+        new Black(new Red(new Black(new Suspend()))).run();
+    }
+
+    class Black implements Runnable {
+        Runnable r;
+
+        public Black(Runnable r) {
+            this.r = r;
+        }
+
+        public void run() {
+            String s = "foo";   // have some random variable
+            r.run();
+            System.out.println(s);
+        }
+    }
+
+    class Red implements Runnable {
+        Runnable r;
+
+        public Red(Runnable r) {
+            this.r = r;
+        }
+
+        public void run() {
+            int i = 5;  // have some random variable
+            r.run();
+            System.out.println(i);
+        }
+    }
+
+    class Suspend implements Runnable {
+        public void run() {
+            Continuation.suspend();
+        }
+    }
+
+}



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message