On Sunday, December 4, 2011, sebb <[email protected]> wrote: > On 4 December 2011 19:59, Philippe Mouawad <[email protected]> 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 <[email protected]> wrote: >> >>> On 4 December 2011 17:45, Philippe Mouawad <[email protected]> >>> 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 <[email protected]> wrote: >>> > >>> >> On 4 December 2011 11:46, <[email protected]> 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.
