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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
