Bug 47290 - Infinite loop on connection factory lookup
Infinite loop on connection factory lookup
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
Nightly (Please specify date)
PC Windows Vista
: P2 normal (vote)
: ---
Assigned To: JMeter issues mailing list
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2009-05-30 15:49 UTC by Peter Johnson
Modified: 2009-06-04 17:20 UTC (History)
0 users



Attachments
Patch for protocol.jms.client.ConnectionFactory (5.82 KB, patch)
2009-05-30 15:49 UTC, Peter Johnson
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Johnson 2009-05-30 15:49:19 UTC
Created attachment 23732 [details]
Patch for protocol.jms.client.ConnectionFactory

I screwed up when I defined the JNDI information for JMS Publisher/Subscriber samplers. When I ran the test script JMeter seemed to hang. I could also hear my disk rattling. I had to kill the process. I went to jmeter.log to see what the problem was an noticed that it was 1GB in size! That lead me to the code in ...protocol.jms.client.ConnectionFactory, where to my amazement there was an infinite loop - the lookup method looped generating error log entries until if found the connection factory. Unfortunately, whoever coded this method originally left no clue as to why there was an infinite loop here.

The patch that is attached assumes that the original coder had a valid reason for retrying the lookup if it failed the first time. But it also places a limit on the amount of time spent in doing a lookup and pauses between each attempt. Personally, I would rather have aborted after the first lookup failed, but I gave the original coder the benefit of the doubt and thus assumed that the retry was done for a purpose.

Needless to say, I added comments that explain exactly why the method is now coded the way it. This will help the next poor soul who stumbles across this code and wonders why it does what it does. I wish that all code was commented this way. I know that I demand it of all the people who work on projects with me.
Comment 1 peter lin 2009-06-04 07:50:58 UTC
Thanks for catching that. The patch looks ok. Is there a reason why the retry in the patch was set so high?

I must have spaced when I wrote that back in 2005.
Comment 2 peter lin 2009-06-04 07:57:16 UTC
From my experience in the past with JMS. Some JMS providers fail on the first try on JNDI lookup. clearly, it should have set a limit on retry, but looks like I spaced and never went back and cleaned up the initial implementation. The sad and funny thing is, I've written jms connection factories several times and had a set number of retries. the bug was there purely due to laziness on my part.
Comment 3 peter lin 2009-06-04 08:05:22 UTC
I've applied the patch and made a note it retries 5 times before giving up.
Comment 4 peter lin 2009-06-04 08:11:15 UTC
changing status to fixed using the patch provided by Peter Johnson.
Comment 5 Sebb 2009-06-04 08:53:40 UTC
SVN details:

URL: http://svn.apache.org/viewvc?rev=781765&view=rev
Log:
apply patch from Peter Johnson. the retry is set to 5 times.
Comment 6 Peter Johnson 2009-06-04 17:20:34 UTC
A retry limit of five times is fine by me, and is what I though would be reasonable when I wrote the patch. That is one of the reasons why I set the limit, and the pause time, as constants.