Bug 50618 - Stack overflow on Generic Controller when IfController is used
Stack overflow on Generic Controller when IfController is used
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
2.5
All All
: P1 regression with 2 votes (vote)
: ---
Assigned To: JMeter issues mailing list
:
: 51272 51690 51768 (view as bug list)
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2011-01-19 10:07 UTC by Luciana Moreira
Modified: 2012-01-22 13:13 UTC (History)
7 users (show)



Attachments
ThreadDump with the stack overflow (77.94 KB, application/octet-stream)
2011-01-19 10:09 UTC, Luciana Moreira
Details
Simple testcase to generate a stack overflow (4.73 KB, application/octet-stream)
2011-01-20 07:32 UTC, Luciana Moreira
Details
File with Include Controller and Sampler (deleted)
2011-01-21 03:45 UTC, Luciana Moreira
Details
A simpler task for a include controller (792 bytes, application/octet-stream)
2011-01-21 03:46 UTC, Luciana Moreira
Details
Excluded CSV and user defined variables (5.54 KB, application/octet-stream)
2011-01-21 05:09 UTC, Luciana Moreira
Details
Simple test to reproduce error (13.88 KB, application/octet-stream)
2011-01-23 14:24 UTC, Milamber
Details
First patch to catch StackOverFlowError (895 bytes, patch)
2011-09-09 18:18 UTC, Milamber
Details | Diff
Fix the StackOverFlow (919 bytes, patch)
2011-09-12 09:52 UTC, Philippe Mouawad
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Luciana Moreira 2011-01-19 10:07:05 UTC
After upgrading to the newest version of JMeter on SVN (revision 1059949), I noticed that I am getting a stack overflow in the following method GenericController.reInitializeSubController().

    /**
     * Called to re-initialize a index of controller's elements (Bug 50032)
     * 
     * @return Sampler
     */
    protected Sampler reInitializeSubController() {
        Sampler returnValue = null;
        try {
            TestElement currentElement = getCurrentElement();
            if (currentElement != null) {
                if (currentElement instanceof Sampler) {
                    returnValue = nextIsASampler((Sampler) currentElement);
                } else { // must be a controller
                    returnValue = nextIsAController((Controller) currentElement);
                    reInitializeSubController();
                }
            }
        } catch (NextIsNullException e) {
        }
        return returnValue;
    }
    
    The problem happens in this else statement:
    } else { // must be a controller
         returnValue = nextIsAController((Controller) currentElement);
         reInitializeSubController();
    }
    
    For some reason when I execute my script I get to this else statement, and from there on, I am not able to leave it any longer. I have attached my ThreadDump in hope that it helps you somehow.
     Any suggestions?
	Luciana
Comment 1 Luciana Moreira 2011-01-19 10:09:05 UTC
Created attachment 26516 [details]
ThreadDump with the stack overflow
Comment 2 Luciana Moreira 2011-01-19 12:20:46 UTC
After searching a bit more I found out that this bug was introduced when fixing bug
Bug 50032.

In the class IfController.java a new line was added by milamber on revision 10215112:
super.reInitializeSubController(); // Bug 50032 - reinitialize current index element for all sub controller

If I comment this out, everything works out like a charm. I do not know however, what is the solution for Bug50032
Comment 3 Milamber 2011-01-19 15:13:11 UTC
Can you provide a test case for this stack overflow?
Thanks
Comment 4 Luciana Moreira 2011-01-20 07:31:50 UTC
Sure. I have attached a simple test case that get's into this condition.
Comment 5 Luciana Moreira 2011-01-20 07:32:33 UTC
Created attachment 26522 [details]
Simple testcase to generate a stack overflow
Comment 6 Milamber 2011-01-20 20:34:02 UTC
This StackOverflowError isn't come with patch of bug 50032.
With your simple testcase, you can have this SOE with the current JMeter 2.4 r961953.

This SOE comes because the IF Controller always returns false (${accept_request} isn't defines in test plan) and thus there haven't any sampler executed.

Your test case is a special case...
If you add a simple sampler (like java sampler) outside the IF Controller (before or after), the SOE is avoided.
Comment 7 Luciana Moreira 2011-01-21 03:45:00 UTC
I simplified my test case to only point to this bug and ended up cutting important parts it would seem...
I have created another test case which has an Include Controller and in there we have a Sampler. In this test case we also get the stack overflow.

In any case, I don't see why we would always need to add a sampler after a If Controller. Can't we have as a last task of a thread group an action executed by a Sampler inside an If Controller? 
Using a big CSV file I can easily imagine many scenarios where this would be needed. According to a variable in the CSV file we do this additional step or not.
Comment 8 Luciana Moreira 2011-01-21 03:45:54 UTC
Created attachment 26527 [details]
File with Include Controller and Sampler
Comment 9 Luciana Moreira 2011-01-21 03:46:28 UTC
Created attachment 26528 [details]
A simpler task for a include controller
Comment 10 Luciana Moreira 2011-01-21 05:09:06 UTC
Created attachment 26529 [details]
Excluded CSV and user defined variables
Comment 11 Mark Thomas 2011-01-21 09:52:32 UTC
The content of attachment 26527 [details] has been deleted by
    Mark Thomas <markt@apache.org>
who provided the following reason:

Inadvertent disclosure of sensitive information

The token used to delete this attachment was generated at 2011-01-21 14:51:02 GMT.
Comment 12 Milamber 2011-01-23 14:23:46 UTC
This bug seems very old. It's reproducible since at least version 2.3.4.
It seems to link to script with no single sampler, only a Simple Controller/If Controller.
I need some time for better understand the root cause. (but I don't have the time from tonight to next month...)
I attached a simple script to reproduce the StackOverFlowError
Comment 13 Milamber 2011-01-23 14:24:43 UTC
Created attachment 26539 [details]
Simple test to reproduce error
Comment 14 Luciana Moreira 2011-02-03 05:41:26 UTC
I just saw that line 173 in the IfController changed from super.reInitializeSubController() to reInitializeSubController().

Even though this is an advance toward a solution in this bug, it does not yet solve the problem. Unfortunatelly the IfController does not implement this method and we once again end up in the GenericController with the stack overflow.

This is unfortunate since I have several tests that used to work perfectly and now are out of operation. Is there any plan on fixing this?
Comment 15 Milamber 2011-02-06 09:44:52 UTC
(In reply to comment #14)
> I just saw that line 173 in the IfController changed from
> super.reInitializeSubController() to reInitializeSubController().
> 
> Even though this is an advance toward a solution in this bug, it does not yet
> solve the problem. 

No, unfortunately, it is not an advance toward a solution.

> Unfortunatelly the IfController does not implement this
> method and we once again end up in the GenericController with the stack
> overflow.
> 
> This is unfortunate since I have several tests that used to work perfectly and
> now are out of operation. Is there any plan on fixing this?

What is the version of JMeter that works with your tests ?

I will works to fix this bug, I just need some time to find the time to do this work.
Comment 16 Luciana Moreira 2011-02-08 09:38:03 UTC
Hello Milamber,

I just synchronized to revision 1021511 on SVN and run my tests without any issues.

Hope this helps.
Luciana
Comment 17 Milamber 2011-02-12 18:21:39 UTC
With revision 1021511, I can reproduce stack overflow with the simple test attached to this bug.
Comment 18 Luciana Moreira 2011-02-16 10:33:03 UTC
Hello Milamber,

I just run your test and it indeed remains with the problem. Nevertheless, in this revision my test case runs with no problem. 

It looks to me that our test cases show 2 different faces of the same problem. Mine had a stack overflow on the IfController, and yours seem to generate this problem in the LoopController.

In revision 1021511 the problem with the IfController was not yet there, and my test cases did not fall into the category that trigger the issue with the LoopController.
Comment 19 Sebb 2011-05-26 20:38:54 UTC
*** Bug 51272 has been marked as a duplicate of this bug. ***
Comment 20 Milamber 2011-08-20 12:05:47 UTC
*** Bug 51690 has been marked as a duplicate of this bug. ***
Comment 21 Christian Seeberger 2011-08-30 23:58:18 UTC
not only the "if" controller is concerned by this bug, but also other controllers following a controller (e.g. while controller). 
I found the following workaround: Just ad a "Test Action" Sampler between the two concerned controllers and configure it with a pause duration of 1ms.
Comment 22 Milamber 2011-09-09 18:18:47 UTC
Created attachment 27478 [details]
First patch to catch StackOverFlowError



This bug comes from a infinite recursive method. JMeter uses next() method to come down the element tree.
If test plan with loops forever (or >2000 loops) contains only a If Controller with children elements, and the IF condition become "false", the StackOverFlowError will start.

The If Controller uses the next() method to provide own feature, if the condition is true, then If Controller next() call super.next() and JMeter calls the first child, if the condition if false, then If Controller next() return null (meaning for JMeter: end of controller).
The issue is providing by GenericController and nextIsController() method which come down in tree recursively, and without stop (i.e. return to JMeterThread) if always null is return by a controller.
With a unique If Controller which start to return always null, the StackOverFlow is inevitable.


A workaround is add a simple sampler like Debug, Test action on the same level of If Controller.

This first patch is to catch the StackOverFlowError and force JMeter to return on JMeterThread (and ignore the SOF error). It's works with the simple test case (TestSimple.jmx) (first iteration return true and the next return always false)

But this patch don't work if the If Controller return always false (from iteration 1 to infinite) (to test it, change If condition to  
"1" == "0" 

@sebb: I would like commit this first patch because it's resolving some case. Are you ok? or any opinion?
Perhaps, we add an "known issue" in documentation?

@all: I spent some hours (day) to find a way to correct this bug. I'm interested by your feedback to help to fix this bug.

Thanks
Comment 23 Sebb 2011-09-09 18:36:23 UTC
(In reply to comment #22)
> Created attachment 27478 [details]
> First patch to catch StackOverFlowError

Patch looks good, but I would use log.warn rather than log.info.
Comment 24 Philippe Mouawad 2011-09-12 07:18:15 UTC
Hello,
For me issue comes from the fact that code in else statement of reInitializeSubController does not change any state, particularly nextIsAController:
nextIsAController((Controller) currentElement);
reInitializeSubController();

so starting from there reInitializeSubController will recurse infinitely.

I attach the same scenario I attached on 50032.

Regards
Philippe
Comment 25 Philippe Mouawad 2011-09-12 07:29:59 UTC
(In reply to comment #24)
> Hello,
> For me issue comes from the fact that code in else statement of
> reInitializeSubController does not change any state, particularly
> nextIsAController:
> nextIsAController((Controller) currentElement);
> reInitializeSubController();
> 
> so starting from there reInitializeSubController will recurse infinitely.
> 
> I attach the same scenario I attached on 50032.
> 
> Regards
> Philippe

Please ignore this comment, my analysis is wrong.
Comment 26 Philippe Mouawad 2011-09-12 07:51:34 UTC
Hello,
Patch may fix the issue but don't you think it can introduce a big performance cost when it occurs ?



Milamber can you explain a little more the reInitializeSubController that fixed the 50032, maybe it could help us help you.


Regards
Philippe
Comment 27 Philippe Mouawad 2011-09-12 08:02:29 UTC
Again,
And a little complement to what I said about catching StackOverflowError:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch. Most such errors are abnormal conditions. The ThreadDeath error, though a "normal" condition, is also a subclass of Error because most applications should not try to catch it. A method is not required to declare in its throws clause any subclasses of Error that might be thrown during the execution of the method but not caught, since these errors are abnormal conditions that should never occur.


Regards
Philippe
Comment 28 Philippe Mouawad 2011-09-12 09:52:12 UTC
Created attachment 27488 [details]
Fix the StackOverFlow

Hello,
Here is my fix proposition.
I just test that nextIsAController does not return null before executing reInitializeSubController a new time.


I tested with my JMX and the JMX in case 50032 and it seems ok.

But once again I think there is still an issue in 50032 fix because it gives wrong results regarding TransactionController inside IfCOntroller, they will be mentionnned as sampled even if condition is false due to what reInitializeSubController does.

Regards
Philippe Mouawad
Comment 29 Milamber 2011-09-13 23:42:54 UTC
(In reply to comment #28)
> Created attachment 27488 [details]
> Fix the StackOverFlow
> 

This patch fixes only infinite loop (without stack overflow error on jmeter.log). This is a regression from bug 50032.
See: https://issues.apache.org/bugzilla/show_bug.cgi?id=50032#c10

SOE already exists with (for example) TestSimple.jmx https://issues.apache.org/bugzilla/attachment.cgi?id=26539
Comment 30 Milamber 2011-09-14 00:02:12 UTC
(In reply to comment #27)
> Again,
> And a little complement to what I said about catching StackOverflowError:
> 
> An Error is a subclass of Throwable that indicates serious problems that a
> reasonable application should not try to catch. Most such errors are abnormal
> conditions. The ThreadDeath error, though a "normal" condition, is also a
> subclass of Error because most applications should not try to catch it. A
> method is not required to declare in its throws clause any subclasses of Error
> that might be thrown during the execution of the method but not caught, since
> these errors are abnormal conditions that should never occur.
> 
> 
> Regards
> Philippe

We are in a special case: infinite recursive loop without exit condition when If controller begin to return null (because the condition return false). (Like a branch on a JMeter tree without end - very big branch)
It's seems very (very) difficult to solve, a way is to introduce a limit on each call (in recursive method), and test this limit to stop loop. This way is not good for JMeter (introduce a limit for If Controller)
With Java, the try catch block is a good way to catch this 'technical' error (the stack is full), and to force JMeter to return on root tree, and start a new turn to fill the stack, etc.

A problem still the test case when If Controller always return false (from first loop). JMeter doesn't count iteration, and start a long running without end...
Comment 31 Milamber 2011-09-14 00:10:07 UTC
(In reply to comment #22)
> Created attachment 27478 [details]
> First patch to catch StackOverFlowError



URL: http://svn.apache.org/viewvc?rev=1170396&view=rev
Log:
If Controller - Catches a StackOverflowError when a condition returns always false (after at least one iteration with return true) See bug 50618

Modified:
    jakarta/jmeter/trunk/src/core/org/apache/jmeter/control/GenericController.java
    jakarta/jmeter/trunk/xdocs/changes.xml
Comment 32 Milamber 2011-09-14 14:42:00 UTC
*** Bug 51768 has been marked as a duplicate of this bug. ***
Comment 33 Milamber 2011-09-14 16:44:52 UTC
URL: http://svn.apache.org/viewvc?rev=1170707&view=rev
Log:
Add two knows issues : bug 50032 and bug 50618

Modified:
    jakarta/jmeter/trunk/xdocs/changes.xml
Comment 34 Philippe Mouawad 2011-12-28 21:47:08 UTC
(In reply to comment #30)
> (In reply to comment #27)
> > Again,
> > And a little complement to what I said about catching StackOverflowError:
> > 
> > An Error is a subclass of Throwable that indicates serious problems that a
> > reasonable application should not try to catch. Most such errors are abnormal
> > conditions. The ThreadDeath error, though a "normal" condition, is also a
> > subclass of Error because most applications should not try to catch it. A
> > method is not required to declare in its throws clause any subclasses of Error
> > that might be thrown during the execution of the method but not caught, since
> > these errors are abnormal conditions that should never occur.
> > 
> > 
> > Regards
> > Philippe
> 
> We are in a special case: infinite recursive loop without exit condition when
> If controller begin to return null (because the condition return false). (Like
> a branch on a JMeter tree without end - very big branch)
> It's seems very (very) difficult to solve, a way is to introduce a limit on
> each call (in recursive method), and test this limit to stop loop. This way is
> not good for JMeter (introduce a limit for If Controller)
> With Java, the try catch block is a good way to catch this 'technical' error
> (the stack is full), and to force JMeter to return on root tree, and start a
> new turn to fill the stack, etc.
> 
> A problem still the test case when If Controller always return false (from
> first loop). JMeter doesn't count iteration, and start a long running without
> end...

Issue seems to be fixed even with always false condition.
Comment 35 Philippe Mouawad 2012-01-22 13:13:45 UTC
As discussed on mailing list, closing issue as this particular one is fixed.
Opened Bug 52496 for counts number that don't increase in Listeners.