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: r1210091 - /jmeter/trunk/src/functions/org/apache/jmeter/functions/StringFromFile.java
Date Sun, 04 Dec 2011 18:39:20 GMT
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.apache.log.Logger;
>> >
>> >  /**
>> > @@ -101,33 +102,13 @@ public class StringFromFile extends Abst
>> >     private String fileName; // needed for error messages
>> >
>> >     public StringFromFile() {
>> > -        init();
>> > +       myValue = ERR_IND;
>> > +       myName = "StringFromFile_";//$NON-NLS-1$
>> >         if (log.isDebugEnabled()) {
>> >             log.debug("++++++++ Construct " + this);
>> >         }
>> >     }
>> >
>> > -    private void init(){
>> > -        myValue = ERR_IND;
>> > -        myName = "StringFromFile_";//$NON-NLS-1$
>> > -    }
>> > -
>> > -    /**
>> > -     * Close file and log
>> > -     */
>> > -    private void closeFile() {
>> > -        if (myBread == null) {
>> > -            return;
>> > -        }
>> > -        String tn = Thread.currentThread().getName();
>> > -        log.info(tn + " closing file " + fileName);//$NON-NLS-1$
>> > -        try {
>> > -            myBread.close();
>> > -        } catch (IOException e) {
>> > -            log.error("closeFile() error: " + e.toString(),
>> e);//$NON-NLS-1$
>> > -        }
>> > -    }
>> > -
>> >     private static final int COUNT_UNUSED = -2;
>> >
>> >     private int myStart = COUNT_UNUSED;
>> > @@ -232,7 +213,7 @@ public class StringFromFile extends Abst
>> >                 if (line == null) { // EOF, re-open file
>> >                     String tn = Thread.currentThread().getName();
>> >                     log.info(tn + " EOF on  file " +
>> fileName);//$NON-NLS-1$
>> > -                    closeFile();
>> > +                    JOrphanUtils.closeQuietly(myBread);
>> >                     openFile();
>> >                     if (myBread != null) {
>> >                         line = myBread.readLine();
>> > @@ -329,7 +310,7 @@ public class StringFromFile extends Abst
>> >
>> >     /** {@inheritDoc} */
>> >     public void testEnded(String host) {
>> > -        closeFile();
>> > +        JOrphanUtils.closeQuietly(myBread);
>> >     }
>> >
>> >     /** {@inheritDoc} */
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message