Is it necessary lock the file when multiple threads try to append the content using NIO in JAVA?

Question

At first I have created an empty file, and then I've invoked some thread to search the database and get the result content, and then append to the file. The result content is String type and may be 20M. Each thread should write into the file one at a time. I have tested many times and I find that it is not necessary to lock. Is that right? The total lines of the example is 1000. When should I need to add a write lock to operate on the file?

    String currentName = "test.txt";
    final String LINE_SEPARATOR = System.getProperty("line.separator");
    ThreadPoolExecutor pool = new ThreadPoolExecutor(
            10, 100, 10, TimeUnit.SECONDS, new LinkedBlockingDeque<Runnable>());
    for (int i = 0; i < 500; i++) {
        pool.execute(() -> {
            try {
                appendFileByFilesWrite(currentName, "abc" +
                        ThreadLocalRandom.current().nextInt(1000) + LINE_SEPARATOR);
            } catch (IOException e) {
                e.printStackTrace();
            }
        });
    }

    IntStream.range(0, 500).<Runnable>mapToObj(a -> () -> {
        try {
            appendFileByFilesWrite( currentName,
                    "def" + ThreadLocalRandom.current().nextInt(1000) +
                    LINE_SEPARATOR);
        } catch (IOException e) {
            e.printStackTrace();
        }
    }).forEach(pool::execute);

    pool.shutdown();

Here is the method:

public static void appendFileByFilesWrite(String fileName,String fileContent) throws IOException {
    Files.write(Paths.get(fileName), fileContent.getBytes(),StandardOpenOption.APPEND);
}

Basically the answer: [Threads and file writing](https://stackoverflow.com/a/9972569/12323248) — akuzminykh, May 27 '20 at 13:08

score 5 · Answer 1 · answered May 27 '20 at 13:07

5

The answer is: always.

Your test works for you. Right now. Today. Maybe during a full moon, it won't. Maybe if you buy a new computer, or your OS vendor updates, or the JDK updates, or you're playing a britney spears song in your winamp, it won't.

The spec says that it is legitimate for the write to be smeared out over multiple steps, and the behaviour of SOO.APPEND is undefined at that point. Possibly if you write 'Hello' and 'World' simultaneously, the file may end up containing 'HelWorllod'. It probably won't. But it could.

Generally, bugs in concurrency are very hard (sometimes literally impossible) to test for. Doesn't make it any less of a bug; mostly you end up with a ton of bug reports, and you answering 'cannot reproduce' on all of them. This is not a good place to be.

Most likely if you want to observe the problem in action, you should write extremely long strings in your writer; the aim is to end up with the actual low-level disk command involving multiple separated out blocks. Even then there is no guarantee that you'll observe a problem. And yet, absence of proof is not proof of absence.

answered May 27 '20 at 13:07

rzwitserloot

85,357
5
51
72

1

…and using a single `Writer` will be more efficient anyway. – Holger May 27 '20 at 14:54
How to use a single Writer?My method is static.Can you give me a example?@Holger – flower May 28 '20 at 01:23
1

@flower when you create a `FileWriter` beforehand, the `appendFileByFilesWrite` method reduces to a simple `writer.write(string)`, in other words, you don’t need a `static` method at all. – Holger Jun 06 '20 at 12:28
@Holger,I write it as a tool class so when I use it in the other class,I do not need to create an instance. – flower Jun 08 '20 at 01:58
1

@flower a tool should be something helpful, not an end in itself. Creating a tool to make the simple call `writer.write(string)` more complicated while producing the problems you hadn’t without, makes no sense. I don’t know why you are so obsessed with the “need to create an instance”, especially for an I/O operation, but *of course*, your tool method creates *more objects* than the alternative of a keeping writer throughout the operations. It starts with the result of the `fileContent.getBytes()`, then `Files.write(…)` doesn’t do magic. It creates an `OuputStream` *on each call*. – Holger Jun 08 '20 at 07:24

score 1 · Answer 2 · answered Feb 19 '21 at 02:03

I use this class when I need to lock a file. It allows for read write locks across multiple JVMs and multiple threads.

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.RandomAccessFile;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.util.Date;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import com.lfp.joe.core.process.CentralExecutor;

public class FileLocks {

    private static final String WRITE_MODE = "rws";
    private static final String READ_MODE = "r";
    private static final Map<String, LockContext> JVM_LOCK_MAP = new ConcurrentHashMap<>();

    private FileLocks() {
    }

    public static <X> X read(File file, ReadAccessor<X> accessor) throws IOException {
        return access(file, false, fc -> {
            try (var is = Channels.newInputStream(fc);) {
                return accessor.read(fc, is);
            }
        });
    }

    public static void write(File file, WriterAccessor accessor) throws IOException {
        access(file, true, fc -> {
            try (var os = Channels.newOutputStream(fc);) {
                accessor.write(fc, os);
            }
            return null;
        });
    }

    public static <X> X access(File file, boolean write, FileChannelAccessor<X> accessor)
            throws FileNotFoundException, IOException {
        Objects.requireNonNull(file);
        Objects.requireNonNull(accessor);
        String path = file.getAbsolutePath();
        var lockContext = JVM_LOCK_MAP.compute(path, (k, v) -> {
            if (v == null)
                v = new LockContext();
            v.incrementAndGetThreadCount();
            return v;
        });
        var jvmLock = write ? lockContext.getAndLockWrite() : lockContext.getAndLockRead();
        try (var randomAccessFile = new RandomAccessFile(file, write ? WRITE_MODE : READ_MODE);
                var fileChannel = randomAccessFile.getChannel();) {
            var fileLock = write ? fileChannel.lock() : null;
            try {
                return accessor.access(fileChannel);
            } finally {
                if (fileLock != null && fileLock.isValid())
                    fileLock.close();
            }
        } finally {
            jvmLock.unlock();
            JVM_LOCK_MAP.compute(path, (k, v) -> {
                if (v == null)
                    return null;
                var threadCount = v.decrementAndGetThreadCount();
                if (threadCount <= 0)
                    return null;
                return v;
            });
        }
    }

    public static interface FileChannelAccessor<X> {

        X access(FileChannel fileChannel) throws IOException;

    }

    public static interface ReadAccessor<X> {

        X read(FileChannel fileChannel, InputStream inputStream) throws IOException;

    }

    public static interface WriterAccessor {

        void write(FileChannel fileChannel, OutputStream outputStream) throws IOException;

    }

    private static class LockContext {

        private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
        private long threadCount = 0;

        public long incrementAndGetThreadCount() {
            threadCount++;
            return threadCount;
        }

        public long decrementAndGetThreadCount() {
            threadCount--;
            return threadCount;
        }

        public Lock getAndLockWrite() {
            var lock = rwLock.writeLock();
            lock.lock();
            return lock;
        }

        public Lock getAndLockRead() {
            var lock = rwLock.readLock();
            lock.lock();
            return lock;
        }
    }
}

You can then use it for writing like so:

File file = new File("test/lock-test.txt");
FileLocks.write(file, (fileChannel, outputStream) -> {
    try (var bw = new BufferedWriter(new OutputStreamWriter(outputStream));) {
        bw.append("cool beans " + new Date().getTime());
    }
});

And reading:

File file = new File("test/lock-test.txt")
var lines = FileLocks.read(file, (fileChannel, inputStream) -> {
    try (var br = new BufferedReader(new InputStreamReader(inputStream));) {
        return br.lines().collect(Collectors.toList());
    }
});

score 0 · Answer 3 · answered May 28 '20 at 04:29

You can use fileLock or just add synchronized to the method.

while (true) {
  try {
  lock = fc.lock();
  break;
   } catch (OverlappingFileLockException e) {
    Thread.sleep(1 * 1000);
   }
 }
appendFileByFilesWrite( fileName, fileContent) ;

or just change like this:

public synchronized  static void appendFileByFilesWrite(String fileName,String fileContent) throws IOException {
    Files.write(Paths.get(fileName), fileContent.getBytes(),StandardOpenOption.APPEND);
}

Is it necessary lock the file when multiple threads try to append the content using NIO in JAVA?

3 Answers3