Then what if we implement testEnded the same way. Looking at previous implementation I don't see why it was done this way.
In my opinion implemented as I did is the best for performances as it cleaned up as soon as it becomes useless, I agree it breaks contract but we could introduce a new parameter : - java.samplerclient.teardown_at_thread_end=true => Would clean at end of thread - java.samplerclient.teardown_at_thread_end=false => Would clean at end of test which would work as today but we would change impl to remove static collection and just call releaseJavaClient() What do you think ? Regards Philippe On Mon, Aug 20, 2012 at 6:13 PM, sebb <[email protected]> wrote: > On 20 August 2012 09:49, <[email protected]> wrote: > > Author: pmouawad > > Date: Mon Aug 20 08:49:59 2012 > > New Revision: 1374946 > > > > URL: http://svn.apache.org/viewvc?rev=1374946&view=rev > > Log: > > Bug 53743 - JavaSamplers.allSamplers static Set keeps references even > after thread has ended > > Bugzilla Id: 53743 > > -1, please revert. > > This does solve the memory issue, but it breaks the contract for > > .JavaSamplerClient#teardownTest(JavaSamplerContext) > > which says: > > "Do any clean-up required by this test at the end of a test run." > > teardownTest is now called at end of thread, not end of test. > > This may break some implementations. > > > Modified: > > > jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java > > jmeter/trunk/xdocs/changes.xml > > > > Modified: > jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java > > URL: > http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java?rev=1374946&r1=1374945&r2=1374946&view=diff > > > ============================================================================== > > --- > jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java > (original) > > +++ > jmeter/trunk/src/protocol/java/org/apache/jmeter/protocol/java/sampler/JavaSampler.java > Mon Aug 20 08:49:59 2012 > > @@ -20,7 +20,6 @@ package org.apache.jmeter.protocol.java. > > > > import java.util.Arrays; > > import java.util.HashSet; > > -import java.util.Iterator; > > import java.util.Set; > > > > import org.apache.jmeter.config.Arguments; > > @@ -31,6 +30,7 @@ import org.apache.jmeter.samplers.Entry; > > import org.apache.jmeter.samplers.SampleResult; > > import org.apache.jmeter.testelement.TestElement; > > import org.apache.jmeter.testelement.TestListener; > > +import org.apache.jmeter.testelement.ThreadListener; > > import org.apache.jmeter.testelement.property.TestElementProperty; > > import org.apache.jorphan.logging.LoggingManager; > > import org.apache.log.Logger; > > @@ -41,7 +41,7 @@ import org.apache.log.Logger; > > * information on writing Java code to be executed by this sampler. > > * > > */ > > -public class JavaSampler extends AbstractSampler implements > TestListener { > > +public class JavaSampler extends AbstractSampler implements > TestListener, ThreadListener { > > > > private static final Logger log = > LoggingManager.getLoggerForClass(); > > > > @@ -76,19 +76,10 @@ public class JavaSampler extends Abstrac > > private transient JavaSamplerContext context = null; > > > > /** > > - * Set used to register all active JavaSamplers. This is used so > that the > > - * samplers can be notified when the test ends. > > - */ > > - private static final Set<JavaSampler> allSamplers = new > HashSet<JavaSampler>(); > > - > > - /** > > * Create a JavaSampler. > > */ > > public JavaSampler() { > > setArguments(new Arguments()); > > - synchronized (allSamplers) { > > - allSamplers.add(this); > > - } > > } > > > > /** > > @@ -240,23 +231,9 @@ public class JavaSampler extends Abstrac > > log.debug(whoAmI() + "\ttestStarted(" + host + ")"); > > } > > > > - /** > > - * Method called at the end of the test. This is called only on one > instance > > - * of JavaSampler. This method will loop through all of the other > > - * JavaSamplers which have been registered (automatically in the > > - * constructor) and notify them that the test has ended, allowing > the > > - * JavaSamplerClients to cleanup. > > - */ > > + /* Implements TestListener.testEnded() */ > > public void testEnded() { > > log.debug(whoAmI() + "\ttestEnded"); > > - synchronized (allSamplers) { > > - Iterator<JavaSampler> i = allSamplers.iterator(); > > - while (i.hasNext()) { > > - JavaSampler sampler = i.next(); > > - sampler.releaseJavaClient(); > > - i.remove(); > > - } > > - } > > } > > > > /* Implements TestListener.testEnded(String) */ > > @@ -300,4 +277,15 @@ public class JavaSampler extends Abstrac > > String guiClass = > configElement.getProperty(TestElement.GUI_CLASS).getStringValue(); > > return APPLIABLE_CONFIG_CLASSES.contains(guiClass); > > } > > + > > + public void threadStarted() { > > + // NOOP > > + } > > + > > + /** > > + * Cleanup java client > > + */ > > + public void threadFinished() { > > + releaseJavaClient(); > > + } > > } > > > > Modified: jmeter/trunk/xdocs/changes.xml > > URL: > http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1374946&r1=1374945&r2=1374946&view=diff > > > ============================================================================== > > --- jmeter/trunk/xdocs/changes.xml (original) > > +++ jmeter/trunk/xdocs/changes.xml Mon Aug 20 08:49:59 2012 > > @@ -84,6 +84,7 @@ JSR223 Test Elements using Script file a > > <li><bugzilla>53357</bugzilla> - JMS Point to Point reports too high > response times in Request Response Mode</li> > > <li><bugzilla>53440</bugzilla> - SSL connection leads to > ArrayStoreException on JDK 6 with some KeyManagerFactory SPI</li> > > <li><bugzilla>53511</bugzilla> - access log sampler SessionFilter > throws NullPointerException - cookie manager not initialized properly</li> > > +<li><bugzilla>53743</bugzilla> - JavaSamplers.allSamplers static Set > keeps references even after thread has ended</li> > > </ul> > > > > <h3>Controllers</h3> > > > > > -- Cordialement. Philippe Mouawad.
