You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ko...@apache.org on 2005/10/08 07:48:06 UTC
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/
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
Re: 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/
Posted by Torsten Curdt <tc...@apache.org>.
> 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.
An explicit runnable makes it possible
to start the stack recording further
down the call stack. We need this for
cocoon were the Invoker is not being
instrumented. So the runnable should
not really be on the stack.
cheers
--
Torsten