jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1210091 - /jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
Date Mon, 05 Dec 2011 06:51:01 GMT
On Sunday, December 4, 2011, sebb <sebbaz@gmail.com> wrote:
> On 4 December 2011 19:59, Philippe Mouawad <philippe.mouawad@gmail.com>
wrote:
>> testEnded is always called by one thread after all threads have ended no
?
>
> Yes, it is a different thread,
>From my understanding this call happens while all thread group threads have
exited so I don' t see how there could be a corruption But maybe i missed
something .
>
>> So I think code is Ok, but it won't harm to add synchronized.
>
> No, the code is not OK as it stands.
>
> That is because of the Java memory model which allows threads to cache
data.
> In the absence of synch (or volatile), an updated value written by one
> thread may perhaps never be seen by another thread.
> Also updates can be seen out of order.
> Some JVMs may even use 2 writes to update a long, in which case other
> threads may see half old/half new values.
>

I agrée but in this particular case where is the issue ? Can you point it
to me just for my full understanding ?

>> My initial  idea was in fact to remove closeFile as I thought logging was
>> not part of a real contract (which is the difference with
>> JOrphanUtils#closeQuietly()).
>
> That was part of the difference in the close() method; the code also
> logged that the file was about to be closed.
>
So for you this info log is part of the méthod contract ?
>> But I let you decide.
>
> I will fix the safe publication issue.
>
I am ok with your fix.

>> Regards
>> Philippe
>>
>> On Sun, Dec 4, 2011 at 7:39 PM, sebb <sebbaz@gmail.com> wrote:
>>
>>> On 4 December 2011 17:45, Philippe Mouawad <philippe.mouawad@gmail.com>
>>> wrote:
>>> > Hello,
>>> > This commit was related to inconsistent synchronization as it removed
>>> code
>>> > where inconsistent synchronisation occured (closeFile), in fact code
is
>>> OK
>>> > but it seems a little inconsistent when only reading closeFile().
>>> >
>>> > I hadn't noticed that closeFile differed from
JOrphanUtils#closeQuietly
>>> > only by logger.error, is it the behaviour you want to keep or I missed
>>> > something ?
>>>
>>> Yes, you also missed the following log message.
>>>
>>>  String tn = Thread.currentThread().getName();
>>>  log.info(tn + " closing file " + fileName);//$NON-NLS-1$
>>>
>>> A simpler solution might have been to make close synchronised.
>>> Unfortunately your fix does not protect all accesses to myBread -
>>> testEnded() also accesses myBread.
>>>
>>> > Regards
>>> > Philipe
>>> >
>>> > On Sun, Dec 4, 2011 at 6:00 PM, sebb <sebbaz@gmail.com> wrote:
>>> >
>>> >> On 4 December 2011 11:46,  <pmouawad@apache.org> wrote:
>>> >> > Author: pmouawad
>>> >> > Date: Sun Dec  4 11:46:18 2011
>>> >> > New Revision: 1210091
>>> >> >
>>> >> > URL: http://svn.apache.org/viewvc?rev=1210091&view=rev
>>> >> > Log:
>>> >> > Bug 52266 - Code:Inconsistent synchronization
>>> >>
>>> >> -1
>>> >>
>>> >> Please don't make multiple unrelated changes in the same commit.
>>> >> Sometimes it's OK to make other minor changes, but then all the
>>> >> changes must be mentioned in the log message.
>>> >> The commit log message should be enough for the reader to understand
>>> >> the SVN diff e-mail.
>>> >>
>>> >> This commit also changes the close file behaviour.
>>> >> The original behaviour was intended; why did you change it?
>>> >>
>>> >> Please restore the close file behaviour.
>>> >>
>>> >> > Modified:
>>> >> >
>>> >>
>>>
 jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
>>> >> >
>>> >> > Modified:
>>> >>
>>>
jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
>>> >> > URL:
>>> >>
>>>
http://svn.apache.org/viewvc/jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java?rev=1210091&r1=1210090&r2=1210091&view=diff
>>> >> >
>>> >>
>>>
==============================================================================
>>> >> > ---
>>> >>
>>>
jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
>>> >> (original)
>>> >> > +++
>>> >>
>>>
jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
>>> >> Sun Dec  4 11:46:18 2011
>>> >> > @@ -35,6 +35,7 @@ import org.apache.jmeter.threads.JMeterV
>>> >> >  import org.apache.jmeter.util.JMeterUtils;
>>> >> >  import org.apache.jorphan.logging.LoggingManager;
>>> >> >  import org.apache.jorphan.util.JMeterStopThreadException;
>>> >> > +import org.apache.jorphan.util.JOrphanUtils;
>>> >> >  import org.apa

-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message