puniverse / quasar

Fibers, Channels and Actors for the JVM
http://docs.paralleluniverse.co/quasar/
Other
4.56k stars 575 forks source link

fiber Stack popMethod -> ArrayIndexOutOfBoundsException #309

Open laughcat opened 6 years ago

laughcat commented 6 years ago

I found a problem:it happens when i call the exchange method of RestTemplate,co.paralleluniverse.fibers.Stack.popMethod throws an ArrayIndexOutOfBoundsException at Stack.java:151, idx equals the length of dataLong.Could you tell me the reason? image

image

doctorpangloss commented 6 years ago

Increase the stack size in the fiber constructor.

doctorpangloss commented 6 years ago

@pron would it be a disaster if dataLong and dataObject (i.e., the actual stack) were just... ArrayLists? CopyOnWriteArrayLists?

rustam9 commented 5 years ago

@doctorpangloss , could you please elaborate? why would increasing the stack size help in this case? I thought the stack was supposed to grow during pushMethod if necessary, maybe we should call growStack in nextMethodEntry too when it bumps ps?

doctorpangloss commented 5 years ago

@rustam9, that's a @pron question why the stack isn't growing in some situations. I've encountered this too, which is why I had the workaround.

I believe that it's possible to push twice but pop once or push once and miss a pop, which would explain why the stack pointer would be too large. I've seen this when examining the stack in a fiber (identical arguments on the stack appearing twice). My bet is that there's something with (1) default methods, (2) overridden methods calling super vs. this, (3) overloaded methods calling other overloads, or some overloads that override and some overloads that do not, especially ones that call each other, or (4) empty methods, especially empty methods that get overridden, especially empty methods in library code that get instrumented and are overridden more than once.

rustam9 commented 5 years ago

Thanks @doctorpangloss for the explanation! The workaround works for me as well. I believe this has been reported at https://groups.google.com/forum/#!topic/quasar-pulsar-user/P6tObzUTLgU with index 96, here it's 48, and in my case it was 192. It can't be just a coincidence that all problematic indices were exactly two folds of 48, and the default stack size is 32+1*16 = 48. I tried to change the following logic in pushMethod:

diff --git a/quasar-core/src/main/java/co/paralleluniverse/fibers/Stack.java b/quasar-core/src/main/java/co/paralleluniverse/fibers/Stack.java
index 70c5821..9d9d1d0 100644
--- a/quasar-core/src/main/java/co/paralleluniverse/fibers/Stack.java
+++ b/quasar-core/src/main/java/co/paralleluniverse/fibers/Stack.java
@@ -131,7 +131,7 @@ public final class Stack implements Serializable {

         int nextMethodIdx = sp + numSlots;
         int nextMethodSP = nextMethodIdx + FRAME_RECORD_SIZE;
-        if (nextMethodSP > dataObject.length)
+        if (nextMethodSP >= dataObject.length)
             growStack(nextMethodSP);

         // clear next method's frame record

The ArrayIndexOutOfBoundsException disappeared without increasing the stack size for Fiber. Even if it's not the right fix, I think it should be pretty close. I will see if I can try to reproduce this with a unit test. @circlespainter @pron

doctorpangloss commented 5 years ago

I think you hit the nail on the head there

rockghost commented 4 years ago

@doctorpangloss @pron @circlespainter Hi, my team is currently on heavy dev job based on Quasar and met the exactly same issue like this one. @rustam9 seems to find a workaround, right?

However could you make a patch or new release with this fix? Actually I know you are focusing project Loom but we don't have enough time for the release of it.