Bug 51840 - JMS : Cache of InitialContext has some issues
JMS : Cache of InitialContext has some issues
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
2.5
All All
: P2 normal (vote)
: ---
Assigned To: JMeter issues mailing list
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2011-09-17 20:16 UTC by Philippe Mouawad
Modified: 2011-09-19 05:27 UTC (History)
1 user (show)



Attachments
Fix to the issue (6.75 KB, patch)
2011-09-18 10:17 UTC, Philippe Mouawad
Details | Diff
Fix to issue (5.99 KB, patch)
2011-09-18 17:27 UTC, Philippe Mouawad
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2011-09-17 20:16:49 UTC
Hello,
I noticed this issue while testing JMS Subscriber and Publisher.
Methode close of InitialContextFactory is never called.
This has the following impacts:
1) Open JMX, run script, close it => When you reopen the old one is used
2) When you rerun a Test, the InitialContext is not reloaded
There is also another biggest issue:
- If you use the same initialContextFactory and providerUrl but one with User /Pwd , the other one without, as these 2 infos are used as a key, this will provoke an issue for one of the samplers depending on which one is initialized first.

Regards
Philippe
Comment 1 Philippe Mouawad 2011-09-17 21:22:00 UTC
(In reply to comment #0)
> Hello,
> I noticed this issue while testing JMS Subscriber and Publisher.
> Methode close of InitialContextFactory is never called.
> This has the following impacts:
> 1) Open JMX, run script, close it => When you reopen the old one is used
> 2) When you rerun a Test, the InitialContext is not reloaded
> There is also another biggest issue:
> - If you use the same initialContextFactory and providerUrl but one with User
> /Pwd , the other one without, as these 2 infos are used as a key, this will
> provoke an issue for one of the samplers depending on which one is initialized
> first.
> 
> Regards
> Philippe

Please ignore this part:
> There is also another biggest issue:
> - If you use the same initialContextFactory and providerUrl but one with User
> /Pwd , the other one without, as these 2 infos are used as a key, this will
> provoke an issue for one of the samplers depending on which one is initialized
> first.

I fixed it in issue patch to issue 51691 where it has its part.
Regards
Philippe
Comment 2 Philippe Mouawad 2011-09-18 10:17:51 UTC
Created attachment 27525 [details]
Fix to the issue

Hello Sebb,
I implemented the TestListener in SubscriberSampler and added in PublisherSampler a call to unregister in testEnded.

By the way I also changed InitialContextFactory synchronization because it seems to me too COARSE grained, due to static synchronized.
I replaced these by a ConcurrentHashMap. 

Regards
Philippe
Comment 3 Sebb 2011-09-18 10:47:13 UTC
Just wondering if it would not be better just to call InitialContextFactory.close() in testEnded() ?

Is there any need to unregister the contexts individually?
Comment 4 Philippe Mouawad 2011-09-18 12:59:25 UTC
I prefer to make each sampler clean its own Contexts so that if some day we have an evolution that requires some Contexts not to be cleared then it will work.
The other thing that bothers me a little is that we will have listener call many times close but only the first one does something.

But I can change it if you think it's better.
Regards
Philippe
Comment 5 Sebb 2011-09-18 13:44:49 UTC
There cannot be any reason to retain any contexts after the end of a test.

Also, what if two samplers use the same context? If contexts are to be individually released, then there would need to be a separate key, or a usage count. That's all more complicated, and unnecessary at present.
Comment 6 Philippe Mouawad 2011-09-18 17:27:56 UTC
Created attachment 27527 [details]
Fix to issue

Hello Sebb,
I changed to call close instead of unregister.
Regards
Philippen,
Comment 7 Sebb 2011-09-19 00:58:45 UTC
Thanks for finding the bug and the patch.

I made two changes to InitialContextFactory.java:
* when the context is added to the map using putIfAbsent, this might return an existing context, in which case we need to return that, rather than the one we just created, or the Map does no correspond with what we have issued.
* simplified the close method - no need to iterate over the map entries when we only want the values.

URL: http://svn.apache.org/viewvc?rev=1172403&view=rev
Log:
Bug 51840 - JMS : Cache of InitialContext has some issues

Modified:
   jakarta/jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/InitialContextFactory.java
   jakarta/jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/PublisherSampler.java
   jakarta/jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/SubscriberSampler.java
   jakarta/jmeter/trunk/xdocs/changes.xml