Name: gm110360 Date: 07/13/2004


FULL PRODUCT VERSION :
java version "1.5.0-beta2"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0-beta2-b51) Java HotSpot(TM) Client VM (build 1.5.0-beta2-b51, mixed mode, sharing)


ADDITIONAL OS VERSION INFORMATION :
Microsoft Windows 2000 [Version 5.00.2195]

A DESCRIPTION OF THE PROBLEM :
BufferedReader.readLine() has a threading issue that causes it to return an empty string when two or more threads are calling readLine().

The file being read has no empty lines ( no blank lines at the end ) and the line it fails on changes from one run to the next.

Only fails on files using \r\n end of lines.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Use the test code below with reading in a file that has no empty lines, preferable a large file.





EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
I would expect because the file has NO empty lines the process would just terminate with no errors.

ACTUAL -

Usually it hits the...

                    System.out.println( "Empty string on first read. " + Thread.currentThread().getName() );

line. Sometimes hitting the exception line but very rarely..

                        throw new Exception( "Invalid tokens with string '" + record + "' on line " + lineCount );


ERROR MESSAGES/STACK TRACES THAT OCCUR :
No exceptions or errors except the ones generated by the test code itself.

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
package com.xxx.test;

import java.io.*;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

/**
 * User: quilleashm
 * Date: 02-Jul-2004
 * Time: 15:02:25
 */
public class TestBufferedReader
{
    public static int lineCount = 0;

    public static void main( String[] args ) throws Exception
    {
        File file = new File( "c:\\temp\\usp1_57261.bcp" );

        BufferedReader reader = new BufferedReader( new FileReader( file ) );

        int threadCount = 2;

        ExecutorService es = Executors.newFixedThreadPool(threadCount);

        for (int i=0; i< threadCount; i++)
            es.execute(new BufferedReaderConsumer(reader));

        // Wait for the tasks to complete
        es.shutdown();
        while (!es.awaitTermination(60, TimeUnit.SECONDS));
    }

    static class BufferedReaderConsumer extends Thread
    {
        BufferedReader reader;

        public BufferedReaderConsumer( BufferedReader reader )
        {
            this.reader = reader;
        }

        public void run()
        {
            try
            {
                String record = reader.readLine();

                if ( record == null )
                {
                    // if the first thread is too fast the second will hit this which is ok
                    System.out.println( "File already finished" );
                    return;
                }

                if ( record.length() == 0 )
                {
                    // usually it comes out here indicating the first read done by
                    // the second thread to run failed
                    System.out.println( "Empty string on first read. " + Thread.currentThread().getName() );
                }

                while ( record != null )
                {
                    lineCount++;
    
                    // Verify the token count
                    if ( record.length() == 0 )
                    {
                        // very occasionally it will fall over here
                        throw new Exception( "Invalid tokens with string '" + record + "' on line " + lineCount );
                    }

                    record = reader.readLine();
                }
            }
            catch ( Exception e )
            {
                e.printStackTrace();
            }
        }
    }
}

---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Quick workaround is to always specify true to readLine( boolean ignoreLF ).

Before I continue it may be that BufferedReader is not intended to be thread safe but the synch locks round most of the functions implies otherwise..

But this appears to be a bug in the implementation of BufferedReader.readLine(). The function uses a boolean member variable called skipLF which is sets whenever it finds a '\r'. At this point skipLF gets set to true and the string/line read up until this point is returned. A subsequent execution will come in and notice that the skipLF is set to true and if the first character is '\n' will ignore it. This is presumably for cross platform compatability for some files ending with \r\n and others with just \r or just \n.

The problem seems to be this line near the top of readLine()


String readLine(boolean ignoreLF) throws IOException
{
    StringBuffer s = null;
    int startChar;
    boolean omitLF = ignoreLF || skipLF; // <<< this line here

    synchronized (lock)
    {
        // rest of the function here



Becuase the omitLF is calculated outside the synch block it is possible for this to happen with 2 threads.

Thread 1 enters the function and acquires the lock.

While Thread1 is doing disk I/O Thread 2 enters the function calculates the omitFL from the ignoreLF ( false ) OR skipLF ( false ) so it's false.

Thread1 finishes it's disk I/O to fill the internal buffer and reads a single line in to a string, reaches a '\r', sets the skipFL to true and exists, releasing the lock.

Thread2 acquires the lock, runs across the first char in the buffer ( '\n' ) which isn't skipped because it calculated it's omitLF flag already outside the lock so it returns an empty line.

The fix is probably to move the omitLF calculuation inside the lock area to prevent the skipLF getting modified after the omitFL has been calculated.


String readLine(boolean ignoreLF) throws IOException
{
    StringBuffer s = null;
    int startChar;

    synchronized (lock)
    {
        boolean omitLF = ignoreLF || skipLF; // <<< move this line here

        // rest of the function here
(Incident Review ID: 282320)
======================================================================