Synchronisation on java.io.File Object

Question

Is it good to use synchronised on java.io.File Object. When you want to alternatively read and write that File Object using two threads: one for reading and one for writing.

public class PrintChar {
    File fileObj;
    public void read() {

    while (true) {
        synchronized (this) {
            readFile();
            notifyAll();
            try {
                wait();
            } catch (InterruptedException e) {
                System.out.println(Thread.currentThread().getName()
                        + " throws Exception");
                e.printStackTrace();
            }
        }
    }
}

public void write(String temp) {

    while (true) {
        synchronized (this) {
            writeFile(temp);
            notifyAll();
            try {
                wait();
            } catch (InterruptedException e) {
                System.out.println(Thread.currentThread().getName()
                        + " throws Exception");
                e.printStackTrace();
            }
        }
    }
}

public void setFileObj(File fileObj) {
    this.fileObj = fileObj;
}

public void readFile() {
    InputStream inputStream;
    try {
        inputStream = new FileInputStream(fileObj);
        // Get the object of DataInputStream
        DataInputStream in = new DataInputStream(inputStream);
        BufferedReader br = new BufferedReader(new InputStreamReader(in));
        String strLine;
        // Read File Line By Line
        while ((strLine = br.readLine()) != null) {
            // Print the content on the console
            System.out.println(strLine);
        }
        in.close();
    } catch (FileNotFoundException e) {
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    }
}

public void writeFile(String temp) {
    BufferedWriter bw;
    try {
        bw = new BufferedWriter(new FileWriter(fileObj, true));
        bw.write(temp);
        bw.newLine();
        bw.close();
    } catch (IOException e) {
        e.printStackTrace();
    }
}

public static void main(String args[]) {

    final PrintChar p = new PrintChar();
    p.setFileObj(new File("C:\\sunny.txt"));

    Thread readingThread = new Thread(new Runnable() {
        @Override
        public void run() {
            p.read();
        }
    });
    Thread writingThread = new Thread(new Runnable() {
        @Override
        public void run() {
            p.write("hello");
        }
    });

    Thread Randomizer = new Thread(new Runnable() {
        @Override
        public void run() {
            while (true)
                try {
                    Thread.sleep(500000);
                } catch (InterruptedException e) {
                    System.out.println(Thread.currentThread().getName()
                            + " throws Exception");
                    e.printStackTrace();
                }
        }
    });

    readingThread.start();
    writingThread.start();
    Randomizer.start();
}

}

In the code above I have used Synchronised(this), Can i use Synchronise(fileObj)??

One More solution I have got from one of my professors is to encapsulate the read and write in objects and push them in a fifo after every operation, if anybody elaborate on this

Answer 1

Edit:

Now that you have added your code, you can lock on fileObj but only if it is not changed. I would move it to the constructor and make it final to make sure that someone doesn't call setFileObj inappropriately. Either that or throw an exception if this.fileObj is not null.

Couple other comments:

• Don't use notifyAll() unless you really need to notify multiple threads.
• If you catch InterruptedException, I'd quit the thread instead of looping. Always make good decisions around catching InterruptedException and don't just print and loop.
• Your in.close(); should be in a finally block.

---

You can lock on any object you want as long as both threads are locking on the same constant object. It is typical to use a private final object for example:

  private final File sharedFile = new File(...);

  // reader
  synchronized (sharedFile) {
       // read from file
  }
  ...

  // writer
  synchronized (sharedFile) {
       // write to file
  }

What you can't do is lock on two different File objects, even if they both point to the same file. The following will not work for example:

  private static final String SHARED_FILE_NAME = "/tmp/some-file";

  // reader
  File readFile = new File(SHARED_FILE_NAME);
  synchronized (readFile) {
      ...
  }

  // writer
  File writeFile = new File(SHARED_FILE_NAME);
  synchronized (writeFile) {
      ...
  }

Also, just because you are locking on the same File object does not mean that the reading and writing code will work between the threads. You will need to make sure that in the writer that all updates are flushed in the synchronized block. In the reader you probably do not want to use buffered streams otherwise you will have stale data.

Answer 2

In general, locking across I/O is not a great idea. It's better to construct your program such that you guarantee by design that usually a given section of the file is not being concurrently written and read, and only lock if you absolutely must mediate between reads and writes of a given piece of the file.

Answer 3

Usually not. There are much better ways: Use a ReentrantLock

This class already offers the "lock for reading/writing" metaphor. It also correctly handles the case that many threads can read at the same time but only one thread can write.

As other people already mentioned, locking will only work if all threads use the same File instance.

Make sure you flush the output buffers after each write; this will cost some performance but otherwise, you'll get stale reads (read thread won't find data that you expect to be there).

If you want to simplify the code, add a third thread which accepts commands from the other two. The commands are READ and WRITE. Put the commands in a queue and let the 3rd thread wait for entries in the queue. Each command should have a callback method (like success()) which the 3rd thread will call when the command has been executed.

This way, you don't need any locking at all. The code for each thread will be much more simple and easy to test.

[EDIT] Answer based on your code: It would work in your case because everyone uses the same instance of fileObj but it would mix several things into one field. People reading your code would expect the file object to be just the path to the file to read. So the solution would violate the principle of least astonishment.

If you'd argue that it would save memory, then I'd reply with "premature optimization".

Try to find a solution which clearly communicates your intent. "Clever" coding is good for your ego but that's about the only positive thing that one can say about it (and it's not good for your ego to learn what people will say about you after they see your "clever" code for the first time...) ;-)

Answer 4

Queueing off read/write objects to one thread that then performs the operation is a valid approach to something, but I'm not sure what.

Wha it would not do, for example, is to enforce read/write/read/write order as you specified in your earlier question. There is nothing to stop the read thread queueing up 100 read requests.

That could be prevented by making the thread that submits an object wait on it until it is signaled by the read/write thread, but this seems a very complex way of just enforcing read/write order, (assuming that's what you still want).

I'm getting to the state now where I'm not sure what it is you need/want.