Locking by value

Question

I have a multi-threaded Java application appending to a variety of files at dynamically generated paths (large numbers -- over 100k). I want to protect against concurrent writes. Because this is contention within the JVM, I can't use the FileLocks.

Instead, I've been trying to synchronize on Path objects as follows (PathLocker is a singleton).

 public class PathLocker {
    private final ConcurrentMap<Path, ReentrantLock> pathLockMap = new ConcurrentHashMap<>();

    public void lock(Path path) {
        pathLockMap.computeIfAbsent(path, p -> new ReentrantLock()).lock();
    }

    public void unlock(Path path) {
        ReentrantLock reentrantLock = pathLockMap.get(path);
        if (!reentrantLock.hasQueuedThreads()) { // NPE OCCURS HERE
            pathLockMap.remove(path);
        }
        reentrantLock.unlock();
    }
}

The only client code looks like this:

Path path = findPath(directory, dataType, bucketEnd, referenceId);
pathLocker.lock(path);
try {
    try (FileWriter fileWriter = new FileWriter(path.toFile(), true)) {
        fileWriter.write(string);
    }
} finally {
    pathLocker.unlock(path);
}

However, this code fairly quickly throws a null pointer when it dereferences reentrantLock within PathLocker::unlock.

I do not understand how this NPE can occur. Clearly, some other thread has removed the value in the meanwhile, but -- as I understand it -- the only possible threads that could remove the lock would be those that were queued and waiting for the lock in the first place. What am I missing?

Answer 1

There is small posibilty that between computeIfAbsent and lock function in thread 1 was invoked hasQueuedThreads function (and return 0) in thread 2. NPE is happening in thread 2 when it finishes it's job and trying to unlock.

If my assumption is correct you should put double barrier in unlock method.

public void unlock(Path path) {
    ReentrantLock reentrantLock = pathLockMap.get(path);
    if (!reentrantLock.hasQueuedThreads()) { // NPE OCCURS HERE
        pathLockMap.remove(path);
        if (reentrantLock.hasQueuedThreads()) { 
            pathLockMap.put(path, reentrantLock);
        }
    }
    reentrantLock.unlock();
}