Looking for an appropriate synchronization mechanism

Question

Presume I have the following (pseudo) code:

class Cache {
    Entry addIfMissing(String data) {
        // omitted for brevity
    }
    void evictOldEntries() {
        // omitted for brevity
    }
}

class Program {
    private Cache cache = new Cache();

    doWork() { // called from multiple threads
        var entry = cache.addIfMissing("omitted for brevity");
        // work with entry
    }

    static {
        Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> {
            cache.evictOldEntries();
        }, 10, 10, TimeUnit.MINUTES);
    }
}

I want to make sure while the evictOldEntries method is running all the other threads in the program have to wait for it to be done.

Which synchronization mechanism would be appropriate in such a scenario?

`I want to make sure while the evictOldEntries method is running all the other threads in the program have to wait for it to be done` -- That is a **lock** or **mutex**. — Robert Harvey, Jun 01 '19 at 17:10
And if your question is "how do I suspend all other threads while evictOldEntries is running," you don't have to do that; you just have to make sure that evictOldEntries is not running when doWork is called. There are ways in java to see if a lock has been acquired. — Robert Harvey, Jun 01 '19 at 17:14
@RobertHarvey I still want to be able to execute `doWork()` in multiple threads concurrently, I just don't know how. If I use a lock only one thread can be inside `doWork()` at once, no? — John Reese, Jun 01 '19 at 17:26
Yes, that's how a lock works. Your problem is not getting your threads to get in line behind doWork; it is blocking doWork while evictOldEntries is executing. — Robert Harvey, Jun 01 '19 at 17:28
Okay, but I am trying to circumvent exactly that scenario. You are right, when evictOldEntries is running no thread should be inside `doWork`. But I want multiple threads to be in `doWork` concurrently. Hence, I think locks aren't appropriate. — John Reese, Jun 01 '19 at 17:32
@JohnReese why do you think that your set-up warrants no locks? Please explain — Hovercraft Full Of Eels, Jun 01 '19 at 17:33
Your lock isn't going to be on doWork; it's going to be on evictOldEntries. There should be a way in Java to detect in doWork whether evictOldEntries has taken a lock. — Robert Harvey, Jun 01 '19 at 17:35
I think you will want accesses to your cache to be mutually exclusive. You don't provide code for your `addIfMissing()` method but even that might be prone to race conditions if accesses to the cache's entries are not properly synchronized. — Eric, Jun 01 '19 at 19:25
What I am doing inside the cache is actually invoking `clang` to compile a C/C++ source file to an `obj` file - which can take quite some time. I really don't want to synchronize the whole access to the cache. — John Reese, Jun 01 '19 at 20:03
Should `evictOldEntries()` be allowed to execute while other threads are executing `addIfMissing()`? If you want fine-grained synchronization, maybe you should look into `ConcurrentHashMap` instead of writing your own cache class. — Eric, Jun 01 '19 at 21:04
@Eric no, I want other threads to wait until `evictOldEntries()` is done. — John Reese, Jun 01 '19 at 21:10
@JohnReese, yes, you want the other threads to wait until `evictOldEntries()` is done but it seems you also don't want to allow `evictOldEntries()` to execute while another thread is calling `addIfMissing()`, according to your comment on Robert Harvey's answer. So it seems that you need mutual exclusion, not just thread signaling. Coarse-grained locking on the entire cache is not acceptable but perhaps entry-level locking is? — Eric, Jun 01 '19 at 21:17

score 3 · Answer 1 · edited Jun 06 '19 at 17:53

What you need is something like this:

class Cache {

    final ReentrantLock lock;

    public Cache { lock = new ReentrantLock(); }

    Entry addIfMissing(String data) {
        lock.lock();
        try {
            // Add data here
        }
        finally {
            lock.unlock();
        }
    }

    void evictOldEntries() {
        if (lock.tryLock()) {
           try {
              // Evict old entries
           }
           finally {
              lock.unlock();
           }
        }
    }
}

score 1 · Answer 2 · answered Jun 01 '19 at 22:04

Taking a bit of liberty here since your code sample and exact requirements are a bit unclear. But might something like this work? ConcurrentHashMap uses fine-grained locking so you minimize the bottleneck when inserting entries into the cache. The evictor thread can even run concurrently with the inserting threads.

class Cache<String, CacheEntry> {
    ConcurrentHashMap<String, CacheEntry> map = new ConcurrentHashMap<String, CacheEntry>();

    Entry addIfMissing(String data) {
        map.computeIfAbsent(...);
    }

    void evictOldEntries() {
        Iterator<Map.Entry<String, CacheEntry>> iterator = map.entrySet().iterator();

        while (iterator.hasNext()) {
            CacheEntry entry = iterator.next().getValue();

            if (shouldEvict(entry)) {
                iterator.remove();
            }
        }
    }
}

`void evictOldEntries() { map.values().removeIf(entry -> shouldEvict(entry)); }` — Holger, Jun 06 '19 at 17:20

score 1 · Answer 3 · answered Jun 01 '19 at 22:56

1

I think ReentrantReadWriteLock is just what I need:

class Program {
    private ReentrantReadWriteLock rwl = new ReentrantReadWriteLock();

    void doWork() { // called from multiple threads
        rwl.readLock().lock();
        try {
            var entry = cache.addIfMissing("omitted for brevity");
            // work with entry
        } finally {
            rwl.readLock().unlock();
        }
    }

    static {
        Executors.newSingleThreadScheduledExecutor().scheduleAtFixedRate(() -> {
            rwl.writeLock().lock();
            try {
                cache.evictOldEntries();
            } finally {
                rwl.writeLock().unlock();
            }
        }, 10, 10, TimeUnit.MINUTES);
    }
}

This way once the writeLock is aquired doWork has to block and vice-versa.

answered Jun 01 '19 at 22:56

John Reese

583
2
6
17

How can you guarantee that writes to your cache in `doWork()` are properly synchronized? – Eric Jun 01 '19 at 23:10
I am using `ConcurrentHashMap` internally inside the Cache itself - I only need a way to mutually exclude `doWork()` and `evictOldEntries()` – John Reese Jun 01 '19 at 23:14
Just curious why isn't the evictor thread not allowed to run concurrently with the other threads? – Eric Jun 01 '19 at 23:31
Essentially the Cache class is a registry of `java.nio.file.Path` objects of `.obj` files on the file system. If `.obj` files are removed, while new compiler processes are spawned, it might happen that the compilation fails. – John Reese Jun 01 '19 at 23:38
The method name `addIfMissing` suggests that the method will modify the cache under certain conditions, so doing it when only owning a `readLock()` is incorrect. – Holger Jun 06 '19 at 17:19

Looking for an appropriate synchronization mechanism

3 Answers3