jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1622838 - in /jmeter/trunk: src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java xdocs/changes.xml
Date Sat, 13 Sep 2014 12:45:28 GMT
On 13 September 2014 13:35, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> Hello Sebb,
> What should we do with this ?
> Do I add volatile ? I would really like to understand why in this case it
> is needed.
>
> AFAIU, if only one thread accesses this value, volatile is not needed. And
> I don't see other thread accessing it.

I have had another look, and it seems I was misled by the comment:

    public void close() { // called from threadFinished() thread

This suggested to me that threadFinished() was being called from a
different thread.

However, I have now checked, and that is not the case;
threadFinished() is called from the current thread.

Sorry for the confusion.

I'll fix the comment

> Thanks
>
> On Sat, Sep 6, 2014 at 6:05 PM, Philippe Mouawad <philippe.mouawad@gmail.com
>> wrote:
>
>>
>>
>> On Saturday, September 6, 2014, sebb <sebbaz@gmail.com> wrote:
>>
>>> On 6 September 2014 15:25, Philippe Mouawad <philippe.mouawad@gmail.com>
>>> wrote:
>>> > On Sat, Sep 6, 2014 at 4:07 PM, sebb <sebbaz@gmail.com> wrote:
>>> >
>>> >> On 6 September 2014 14:30, Philippe Mouawad <
>>> philippe.mouawad@gmail.com>
>>> >> wrote:
>>> >> > Hello sebb,
>>> >> > I have a doubt, 2 questions:
>>> >> > 1) For me there is one instance of ReceiveSubscriber per thread
and
>>> no
>>> >> > concurrent access
>>> >>
>>> >> As the comment says:
>>> >>
>>> >> public void close() { // called from threadFinished() thread
>>> >>
>>> >
>>> > Agree but it is still the same JMeterThread that called sample, so no
>>> > concurrent threads accessing for me.
>>> > I debugged it and couldn't find.
>>>
>>> Agreed, but that is not the only reason why sync is needed.
>>
>> can you explain a bit more in this case, I don't get it
>>
>>>
>>> >>
>>> >> > 2)Even if there was , this is more of a question as I have a doubt,
>>> if
>>> >> the
>>> >> > access is sequential, is volatile still needed ?
>>> >>
>>> >> The Java memory model allows threads to cache variables.
>>> >>
>>> >> This is not about concurrency, it's about safe publication of data.
>>> >>
>>> >> Volatile is needed here to ensure memory caches are properly
>>> >> flushed/refreshed.
>>> >>
>>> >> Ok
>>> >
>>> >> > Thanks
>>> >> >
>>> >> >
>>> >> > On Sat, Sep 6, 2014 at 2:32 PM, sebb <sebbaz@gmail.com> wrote:
>>> >> >
>>> >> >> On 6 September 2014 10:50,  <pmouawad@apache.org> wrote:
>>> >> >> > Author: pmouawad
>>> >> >> > Date: Sat Sep  6 09:50:18 2014
>>> >> >> > New Revision: 1622838
>>> >> >> >
>>> >> >> > URL: http://svn.apache.org/r1622838
>>> >> >> > Log:
>>> >> >> > Bug 56761 - JMeter tries to stop already stopped JMS connection
>>> and
>>> >> >> displays "The connection is closed"
>>> >> >> > Bugzilla Id: 56761
>>> >> >> >
>>> >> >> > Modified:
>>> >> >> >
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java
>>> >> >> >     jmeter/trunk/xdocs/changes.xml
>>> >> >> >
>>> >> >> > Modified:
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java
>>> >> >> > URL:
>>> >> >>
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java?rev=1622838&r1=1622837&r2=1622838&view=diff
>>> >> >> >
>>> >> >>
>>> >>
>>> ==============================================================================
>>> >> >> > ---
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java
>>> >> >> (original)
>>> >> >> > +++
>>> >> >>
>>> >>
>>> jmeter/trunk/src/protocol/jms/org/apache/jmeter/protocol/jms/client/ReceiveSubscriber.java
>>> >> >> Sat Sep  6 09:50:18 2014
>>> >> >> > @@ -62,6 +62,8 @@ public class ReceiveSubscriber implement
>>> >> >> >       */
>>> >> >> >      private final LinkedBlockingQueue<Message>
queue;
>>> >> >> >
>>> >> >> > +    private boolean connectionStarted;
>>> >> >>
>>> >> >> This needs to be volatile, as it is accessed from multiple
threads.
>>> >> >>
>>> >> >> > +
>>> >> >> >      /**
>>> >> >> >       * Constructor takes the necessary JNDI related parameters
to
>>> >> >> create a
>>> >> >> >       * connection and prepare to begin receiving messages.
>>> >> >> > @@ -222,6 +224,7 @@ public class ReceiveSubscriber implement
>>> >> >> >      public void start() throws JMSException {
>>> >> >> >          log.debug("start()");
>>> >> >> >          connection.start();
>>> >> >> > +        connectionStarted=true;
>>> >> >> >      }
>>> >> >> >
>>> >> >> >      /**
>>> >> >> > @@ -231,6 +234,7 @@ public class ReceiveSubscriber implement
>>> >> >> >      public void stop() throws JMSException {
>>> >> >> >          log.debug("stop()");
>>> >> >> >          connection.stop();
>>> >> >> > +        connectionStarted=false;
>>> >> >> >      }
>>> >> >> >
>>> >> >> >      /**
>>> >> >> > @@ -271,11 +275,12 @@ public class ReceiveSubscriber implement
>>> >> >> >      public void close() { // called from threadFinished()
thread
>>> >> >> >          log.debug("close()");
>>> >> >> >          try {
>>> >> >> > -            if(connection != null) {
>>> >> >> > +            if(connection != null && connectionStarted)
{
>>> >> >> >                  connection.stop();
>>> >> >> > +                connectionStarted = false;
>>> >> >> >              }
>>> >> >> >          } catch (JMSException e) {
>>> >> >> > -            log.error(e.getMessage());
>>> >> >> > +            log.warn("Stopping connection throws exception,
>>> >> >> message:"+e.getMessage());
>>> >> >> >          }
>>> >> >> >          Utils.close(subscriber, log);
>>> >> >> >          Utils.close(session, log);
>>> >> >> >
>>> >> >> > Modified: jmeter/trunk/xdocs/changes.xml
>>> >> >> > URL:
>>> >> >>
>>> >>
>>> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1622838&r1=1622837&r2=1622838&view=diff
>>> >> >> >
>>> >> >>
>>> >>
>>> ==============================================================================
>>> >> >> > --- jmeter/trunk/xdocs/changes.xml (original)
>>> >> >> > +++ jmeter/trunk/xdocs/changes.xml Sat Sep  6 09:50:18
2014
>>> >> >> > @@ -154,6 +154,7 @@ jmeter.gui.action.LookAndFeelCommand:
Us
>>> >> >> >  <li><bugzilla>46932</bugzilla> - Alias
given in select statement
>>> is
>>> >> not
>>> >> >> used as column header in response data for a JDBC request.Based
on
>>> >> report
>>> >> >> and analysis of Nicola Ambrosetti</li>
>>> >> >> >  <li><bugzilla>56539</bugzilla> - Mail
reader sampler: When
>>> Number of
>>> >> >> messages to retrieve is superior to 1, Number of samples should
only
>>> >> show 1
>>> >> >> not the number of messages retrieved</li>
>>> >> >> >  <li><bugzilla>56809</bugzilla> - JMSSampler
closes
>>> InitialContext too
>>> >> >> early. Contributed by Bradford Hovinen (hovinen at gmail.com)</li>
>>> >> >> > +<li><bugzilla>56761</bugzilla> - JMeter
tries to stop already
>>> stopped
>>> >> >> JMS connection and displays "The connection is closed"</li>
>>> >> >> >  </ul>
>>> >> >> >
>>> >> >> >  <h3>Controllers</h3>
>>> >> >> > @@ -233,8 +234,8 @@ jmeter.gui.action.LookAndFeelCommand:
Us
>>> >> >> >  <ul>
>>> >> >> >  <li><bugzilla>56691</bugzilla> - Synchronizing
Timer : Add
>>> timeout on
>>> >> >> waiting</li>
>>> >> >> >  <li><bugzilla>56701</bugzilla> - HTTP
Authorization Manager/
>>> Kerberos
>>> >> >> Authentication: add port to SPN when server port is neither
80 nor
>>> 443.
>>> >> >> Based on patches from Dan Haughey (dan.haughey at swinton.co.uk)
>>> and
>>> >> >> Felix Schumacher (felix.schumacher at internetallee.de)</li>
>>> >> >> > -<li><bugzilla>56841</bugzilla> - New
configuration element: DNS
>>> Cache
>>> >> >> Manager to improve the testing of CDN. Based on patch from
Dzmitry
>>> >> Kashlach
>>> >> >> (dzmitrykashlach at gmail.com)</li>
>>> >> >> > -<li><bugzilla>52061</bugzilla> - Allow
access to Request Headers
>>> in
>>> >> >> Regex Extractor. Based on patch from Dzmitry Kashlach
>>> (dzmitrykashlach
>>> >> at
>>> >> >> gmail.com)</li>
>>> >> >> > +<li><bugzilla>56841</bugzilla> - New
configuration element: DNS
>>> Cache
>>> >> >> Manager to improve the testing of CDN. Based on patch from
Dzmitry
>>> >> Kashlach
>>> >> >> (dzmitrykashlach at gmail.com), donated by BlazeMeter Ltd.</li>
>>> >> >> > +<li><bugzilla>52061</bugzilla> - Allow
access to Request Headers
>>> in
>>> >> >> Regex Extractor. Based on patch from Dzmitry Kashlach
>>> (dzmitrykashlach
>>> >> at
>>> >> >> gmail.com), donated by BlazeMeter Ltd.</li>
>>> >> >> >  </ul>
>>> >> >> >
>>> >> >> >  <h3>Functions</h3>
>>> >> >> > @@ -274,13 +275,14 @@ jmeter.gui.action.LookAndFeelCommand:
Us
>>> >> >> >  <li>James Liang (jliang at andera.com)</li>
>>> >> >> >  <li>Emmanuel Bourg (ebourg at apache.org)</li>
>>> >> >> >  <li>Nicola Ambrosetti (ambrosetti.nicola at gmail.com)</li>
>>> >> >> > -<li><a href="http://ubikloadpack.com">Ubik
Load Pack
>>> >> support</a></li>
>>> >> >> > +<li><a href="http://ubikloadpack.com">Ubik
Load Pack</a></li>
>>> >> >> >  <li>Mikhail Epikhin (epihin-m at yandex.ru)</li>
>>> >> >> >  <li>Dan Haughey (dan.haughey at swinton.co.uk)</li>
>>> >> >> >  <li>Felix Schumacher (felix.schumacher at internetallee.de)</li>
>>> >> >> >  <li>Dzmitry Kashlach (dzmitrykashlach at gmail.com)</li>
>>> >> >> >  <li>Andrey Pohilko (apc4 at ya.ru)</li>
>>> >> >> >  <li>Bradford Hovinen (hovinen at gmail.com)</li>
>>> >> >> > +<li><a href="http://blazemeter.com">BlazeMeter
Ltd.</a></li>
>>> >> >> >  </ul>
>>> >> >> >
>>> >> >> >  <br/>
>>> >> >> >
>>> >> >> >
>>> >> >>
>>> >> >
>>> >> >
>>> >> >
>>> >> > --
>>> >> > Cordialement.
>>> >> > Philippe Mouawad.
>>> >>
>>> >
>>> >
>>> >
>>> > --
>>> > Cordialement.
>>> > Philippe Mouawad.
>>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
>>
>>
>>
>>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message