Thread safety and performance when multiple threads write into same Writer

Question

Multiple threads write data to a file. When maxLines is reached, the file is closed and a new file is created:

public synchronized void writeLine(String data) throws IOException {
  writer.write(data + "\n");
  lineCounter++;

  if(maxLines != 0 && lineCounter == maxLines) {
    close();

    lineCounter = 0;
    fileCounter++;

    addFile();
    open();
  }
}

As you can see I currently solved this with a synchronized method. This ensures that thread 1 won't write when thread 2 just closed the writer.

I think that's pretty slow. Is there a better solution for this? Maybe with lineCounter as volatile?

Answer 1

Making the lineCounter volatile will not help here. If synchronized is not used, the following sequence of actions is possible:

1. Thread A writes something to a file.

2. Thread B writes something to a file.

If the lineCounter was equal to maxLines - 1 before the first step, the behavior of the program is obviously incorrect.

As you have said, you need to synchronize a group of actions(write, close, addFile, open). That's why you need to use synchronized(or a Lock).

About performance: are you sure that synchronization is really an issue? Did you profile your code or are you just making guesses? Before making things more complicated, you should make sure that it will actually solve the problem.

Answer 2

In addition to why a synchronized block is needed, making the method synchronized is not the best implementation. Using the synchronized modifier on a method synchronizes the method on the class instance. Any code can also synchronize on these instances, potentially causing deadlocks in code you don't control. You should always synchronize on a private final instance that only you control.

Answer 3

This problem can easily be solved using the Producer-Consumer pattern. Thread A & B from your example are the producers (they produce the tasks : lines to be written to a file). A third thread C, the consumer, will take these tasks, and process them (do the actual writing). Producers hand tasks to the consumer through a thread safe queue.

Typically you'll want a bounded and blocking queue. This way producers will stop producing tasks when the consumer cant' keep up, and the consumer will stop processing when the producers can't keep up.

Synchronization concerns are limited to using a thread safe queue implementation and making sure the tasks are thread safe. As String is immutable that is not a problem, and the JDK comes with various flavours of thread safe queues, so no need to roll your own. (ArrayBlockingQueue should serve just fine, for instance).