You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@jmeter.apache.org by bu...@apache.org on 2016/01/03 15:53:29 UTC
[Bug 58790] New: Issue in CheckDirty and its relation to
ActionRouter
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
Bug ID: 58790
Summary: Issue in CheckDirty and its relation to ActionRouter
Product: JMeter
Version: 2.13
Hardware: All
OS: All
Status: NEW
Severity: minor
Priority: P2
Component: Main
Assignee: issues@jmeter.apache.org
Reporter: p.mouawad@ubik-ingenierie.com
Currently there is an issue in ActionRouter#getInstance()
It returns a partially initialized object so a fix would be to use a local
object and affect it :
public static ActionRouter getInstance() {
if (router == null) {
synchronized (LOCK) {
if(router == null) {
ActionRouter router = new ActionRouter();
router.populateCommandMap();
ActionRouter.router = router;
}
}
}
return router;
}
But due to CheckDirty using getInstance() in its constructor to register itself
as a PreAction to ExitCommand, it leads to a StackoverflowError.
Even without fixing this issue, it reveals an issue that could occur if a
Constructor of an AbstractCommand subclass uses ActionRouter.getInstance()
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #1 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #0)
> Currently there is an issue in ActionRouter#getInstance()
> It returns a partially initialized object so a fix would be to use a local
> object and affect it :
> public static ActionRouter getInstance() {
> if (router == null) {
> synchronized (LOCK) {
> if(router == null) {
> ActionRouter router = new ActionRouter();
> router.populateCommandMap();
> ActionRouter.router = router;
> }
> }
> }
> return router;
> }
Alternatively one could use the IODH idiom.
>
> But due to CheckDirty using getInstance() in its constructor to register
> itself as a PreAction to ExitCommand, it leads to a StackoverflowError.
Not sure I follow this.
> Even without fixing this issue, it reveals an issue that could occur if a
> Constructor of an AbstractCommand subclass uses ActionRouter.getInstance()
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #6 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #5)
> (In reply to Sebb from comment #3)
> > (In reply to Philippe Mouawad from comment #2)
> > > > > But due to CheckDirty using getInstance() in its constructor to register
> > > > > itself as a PreAction to ExitCommand, it leads to a StackoverflowError.
> > > >
> > > > Not sure I follow this.
> > > Try you will see
> >
> > Try what exactly?
> >
> > JMeter works fine for me if I try to exit without saving an updated test
> > plan.
>
> Try the code that I wrote and run the junit tests.
Sorry, I see now, you meant that the fix causes the Stack Overflow.
IODH won't help here.
A possible work-round is for ActionRouter to add CheckDirty as a PreAction
Listener after it has finished populating the command map. i.e. we ban the use
of getInstance from classes that implement Command. I don't like that.
Another might be to get org.apache.jmeter.JMeter.startGui to run the
populateCommandMap code instead of doing it in getInstance. AFAICT that is the
first place where ActionRouter is created. This would mean making
populateCommandMap public, but it could protect itself from repeated
invocations by having a done flag (or seeing if there are any commands). A
quick test shows this seems to work.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #7 from Sebb <se...@apache.org> ---
Fixed:
URL: http://svn.apache.org/viewvc?rev=1723468&view=rev
Log:
Issue in CheckDirty and its relation to ActionRouter
Bugzilla Id: 58790
Modified:
jmeter/trunk/src/core/org/apache/jmeter/JMeter.java
jmeter/trunk/src/core/org/apache/jmeter/gui/action/ActionRouter.java
jmeter/trunk/xdocs/changes.xml
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #5 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Sebb from comment #3)
> (In reply to Philippe Mouawad from comment #2)
> > > > But due to CheckDirty using getInstance() in its constructor to register
> > > > itself as a PreAction to ExitCommand, it leads to a StackoverflowError.
> > >
> > > Not sure I follow this.
> > Try you will see
>
> Try what exactly?
>
> JMeter works fine for me if I try to exit without saving an updated test
> plan.
Try the code that I wrote and run the junit tests.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #3 from Sebb <se...@apache.org> ---
(In reply to Philippe Mouawad from comment #2)
> > > But due to CheckDirty using getInstance() in its constructor to register
> > > itself as a PreAction to ExitCommand, it leads to a StackoverflowError.
> >
> > Not sure I follow this.
> Try you will see
Try what exactly?
JMeter works fine for me if I try to exit without saving an updated test plan.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
--- Comment #4 from Sebb <se...@apache.org> ---
Note: I agree that the code needs to be changed because there's a clear
threading issue. However I have not seen any related failures.
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |p.mouawad@ubik-ingenierie.c
| |om
--- Comment #2 from Philippe Mouawad <p....@ubik-ingenierie.com> ---
(In reply to Sebb from comment #1)
> (In reply to Philippe Mouawad from comment #0)
> > Currently there is an issue in ActionRouter#getInstance()
> > It returns a partially initialized object so a fix would be to use a local
> > object and affect it :
> > public static ActionRouter getInstance() {
> > if (router == null) {
> > synchronized (LOCK) {
> > if(router == null) {
> > ActionRouter router = new ActionRouter();
> > router.populateCommandMap();
> > ActionRouter.router = router;
> > }
> > }
> > }
> > return router;
> > }
>
> Alternatively one could use the IODH idiom.
>
> >
> > But due to CheckDirty using getInstance() in its constructor to register
> > itself as a PreAction to ExitCommand, it leads to a StackoverflowError.
>
> Not sure I follow this.
Try you will see
>
> > Even without fixing this issue, it reveals an issue that could occur if a
> > Constructor of an AbstractCommand subclass uses ActionRouter.getInstance()
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 58790] Issue in CheckDirty and its relation to ActionRouter
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58790
Philippe Mouawad <p....@ubik-ingenierie.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--
You are receiving this mail because:
You are the assignee for the bug.