How to implement such use case by Java Concurrency Lock

Question

In the test case below, the update() method is called outside of the class which in running for every hour by one thread only. But there have multiple threads calling getKey1() and getKey2() at the same time.

So for this use case:

The most important is since getKey1() and getKey2() are calling almost at the same time, we must make sure

if flag is true, update both key1 and key2 and return the new key
if not, get the old key1 and key2

We donot want such case exist: update the key1 and get the old key2, but it's ok to get the old key1 and key2 for few request even we already update it.

public class Test {

    private Lock lock = new ReentrantLock();

    private AtomicBoolean flag1 = new AtomicBoolean(false);
    private AtomicBoolean flag2 = new AtomicBoolean(false);

    private volatile String key1;
    private volatile String key2;

    public Test(String key1, String key2) {
        this.key1 = key1;
        this.key2 = key2;
    }

    public void update() {
        lock.lock();
        try {
            flag1.compareAndSet(false, true);
            flag2.compareAndSet(false, true);
        } finally {
            lock.unlock();
        }
    }

    public String getKey1() {
        // TODO if the lock is holding... block over here
        if (flag1.get()) {
            // doing something for key 1
            key1 = getFromFile();
            flag1.set(false);
        }
        return key1;
    }

    public String getKey2() {
        // TODO if the lock is holding... block over here
        if (flag2.get()) {
            // doing something for key 1
            key2 = getFromFile();
            flag2.set(false);
        }
        return key1;
    }
}

My idea is:

when update() is running, block both getKey1() and getKey2(), waiting to get the update key
when update() is not running, getKey1() and getKey2() should both be ok and return directly.
when calling either getKey1() or getKey2(), I think we do not need to block the update() method.

Anyone have any ideas about the implementation?

Are you sure `ReentrantLock` is what you need? Have you considered a [ReadWriteLock](https://stackoverflow.com/questions/18354339/reentrantreadwritelock-whats-the-difference-between-readlock-and-writelock/18354623) — OldCurmudgeon, Jun 07 '17 at 05:59

score 0 · Answer 1 · answered Jun 07 '17 at 06:21

As already said java.util.concurrent.locks.ReentrantReadWriteLock suits you best.

To put it in your example :

import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class Test {

    private ReadWriteLock lock = new ReentrantReadWriteLock();

    private AtomicBoolean flag1 = new AtomicBoolean(false);
    private AtomicBoolean flag2 = new AtomicBoolean(false);

    private volatile String key1;
    private volatile String key2;

    public Test(String key1, String key2) {
        this.key1 = key1;
        this.key2 = key2;
    }

    public void update() {
        Lock writeLock = lock.writeLock();
        try {
            flag1.compareAndSet(false, true);
            flag2.compareAndSet(false, true);
        } finally {
            writeLock.unlock();
        }
    }

    public String getKey1() {
        Lock readLock = lock.readLock();
        try {
            if (flag1.get()) {
                // doing something for key 1
                key1 = getFromFile();
                flag1.set(false);
            }
            return key1;
        } finally {
            readLock.unlock();
        }
    }

    public String getKey2() {
        Lock readLock = lock.readLock();
        try {
            if (flag2.get()) {
                // doing something for key 1
                key2 = getFromFile();
                flag2.set(false);
            }
            return key1;
        } finally {
            readLock.unlock();
        }
    }
}

From docs of ReadWriteLock interface:

The read lock may be held simultaneously by multiple reader threads, so long as there are no writers. The write lock is exclusive.

`getKey1()` and `getKey2()` usually be called at the same time, but what if the update happened between the `getKey1()` and `getKey2()`, which it will read the old key1 but newest key2? Is there any better solution to avoid that (getKey1() and getKey2() is override from an interface so we cannot combine both into one method. — Jie, Jun 11 '17 at 02:51
Does the flag1 and flag2 still be AtomicBoolean? key1 and key2 still be volatile since it already within the lock. Actually is it possible to combine the flag1 and flag2 into one field? — Jie, Jun 11 '17 at 02:52
After lots of internet search I found [this answer](https://stackoverflow.com/a/27751714/6743203). Create a wrapper of `key1` and `key2` and use [`AtomicReference`](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicReference.html) to update them atomically. — Jay Smith, Jun 11 '17 at 19:02

score 0 · Answer 2 · answered Jun 07 '17 at 06:40

An update on the below ReadWriteLock sample, we have to acquire locks before starting the read/write operations. As I don't have permission to edit the sample, posting a new answer.

import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

public class Test {

    private ReadWriteLock lock = new ReentrantReadWriteLock();

    private AtomicBoolean flag1 = new AtomicBoolean(false);
    private AtomicBoolean flag2 = new AtomicBoolean(false);

    private volatile String key1;
    private volatile String key2;

    public Test(String key1, String key2) {
        this.key1 = key1;
        this.key2 = key2;
    }

    public void update() {
        Lock writeLock = lock.writeLock();
        try {
            writeLock.lock(); 
            flag1.compareAndSet(false, true);
            flag2.compareAndSet(false, true);
        } finally {
            writeLock.unlock();
        }
    }

    public String getKey1() {
        Lock readLock = lock.readLock();
        try {
            readLock.lock(); 
            if (flag1.get()) {
                // doing something for key 1
                key1 = getFromFile();
                flag1.set(false);
            }
            return key1;
        } finally {
            readLock.unlock();
        }
    }

    public String getKey2() {
        Lock readLock = lock.readLock();
        try {
            readLock.lock();
            if (flag2.get()) {
                // doing something for key 1
                key2 = getFromFile();
                flag2.set(false);
            }
            return key1;
        } finally {
            readLock.unlock();
        }
    }
}

getKey1() and getKey2() usually be called at the same time, but what if the update happened between the getKey1() and getKey2(), which it will read the old key1 but newest key2? Is there any better solution to avoid that (getKey1() and getKey2() is override from an interface so we cannot combine both into one method. — Jie, Jun 11 '17 at 02:52

Persixty · Answer 3 · 2017-06-07T07:25:19.707

This is certainly a case where a ReadWriteLock should help. Many concurrent readers can operate with miniscule contention and very infrequently a writer acquires the write-lock and updates.

However the key issue raised is that a thread must not read one old key and one new key in a single pass.

That's a classical problem. The easiest solution is to update both keys in the update thread.

ReentrantReadWriteLock rwlock=new ReentrantReadWriteLock(true);
//If in doubt require a fair lock to avoid live-locking writes...

//...

public void update() {
    String new1=getFromFile();
    String new2=getFromFile();
    //That's the heavy lifting. Only now acquire the lock...
    Lock wlock=rwlock.writeLock();
    wlock.lock();
    try {
        key1=new1;
        key2=new2;
    } finally {
        wlock.unlock();
    }
}

And in the readers:

public String[] getKeys() {
    String k1;
    String k2;
    Lock rlock=rwlock.readLock();
    rlock.lock();
    try {
        k1=key1;
        k2=key2;
    } finally {
        rlock.unlock();
    }    
    String[] r={k1,k2};
    return r;
 }

A read-write lock has two connected locks. Multiple readers can acquire the read-lock but the write lock is exclusive of readers and other writers. It is a common scenario and has established solutions.

It may be important to get from the file before acquiring the lock as I/O is often relatively slow and you should hold the lock for the minimum possible time.

It's also important to return both keys from the same acquisition of the lock. There's no other easy way to stop update happening between calls to getKey1() and getKey2().

You no longer need to use the volatile keyword because of the memory barrier guarantees of the lock (see doc.).

You don't actually need a re-entrant lock but that is what is provided out-of-the-box for a read-write lock.

This is a case where something called a sequential lock (Seqlock) could help but that will involve more coding and may be over-engineered here.

References:

https://en.wikipedia.org/wiki/Seqlock

https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReadWriteLock.html

The problem I am facing is this class is implement from an interface. So `getKey1()` and `getKey2()` is override method that I cannot combine them into one method. The update method is find, I will update the file outside of the class and then do the writeLock — Jie, Jun 11 '17 at 02:54
@Jie n which case based on the interface presented there's no way of ensuring that `update()` can't be run between `getKey1()` and `getKey2()`. So whatever you do you'll risk reading a old key then a new key. You might find some way of suspending writes between linked calls to `getKey1()` and `getKey2()` but there's not enough code in the question to see where that is. There may be a hack (lock early in `getKey1()` and unlock late in `getKey2()`) but that will depend heavily on the code calling the interface. — Persixty, Jun 11 '17 at 10:57

Kajal · Answer 4 · 2017-06-08T10:21:37.143

-2

Use tryLock()

public String getKey1() {
  if (lock.tryLock())
  {
    // Got the lock
    try
    {
        // Process record
    }
    finally
    {
        // Make sure to unlock so that we don't cause a deadlock
        lock.unlock();
    }
 }
}

If you don't want getKey1() or getKey2() to perform locking. You can use the following approach,

static boolean isLocked=false;
public void update() {
        lock.lock();
        isLocked = true;
        try {
            flag1.compareAndSet(false, true);
            flag2.compareAndSet(false, true);
        } finally {
            lock.unlock();
            isLocked = false; 
        }
    }

    public String getKey1() {
        while(isLocked); #wait till the lock release.
        if (flag1.get()) {
            // doing something for key 1
            key1 = getFromFile();
            flag1.set(false);
        }
        return key1;
    }

Ref : how-do-determine-if-an-object-is-locked-synchronized-so-not-to-block-in-java

edited Jun 08 '17 at 10:21

answered Jun 07 '17 at 05:55

Kajal

709
8
27

You should (at least) declare `isLocked` to be `volatile` because there is no guarantee that the reader thread will see its value change. There are implicit memory barriers in the accesses to the other `volatiles` but they're after or before the changes to `isLocked` so not helping. That said I don't understand what this achieves. If the lock is acquired after `while(isLocked);` the body of `getKey1()` could still execute while the lock is held. – Persixty Jun 07 '17 at 10:52
Thanks Persixty3. It should be volatile. – Kajal Jun 07 '17 at 14:28
I still don't see what that code achieves. The lock may be acquired at any time during that method. – Persixty Jun 08 '17 at 10:16
Lock can be achieved. But while performing update action getKey functions will wait till the lock get released. – Kajal Jun 08 '17 at 10:24
But it can be acquired during the method. That codes doesn't ensure that the lock is not being held during the read. It doesn't ensure synchronization. – Persixty Jun 08 '17 at 10:27
Yes, here only the update method is synch. As per my understanding of his question getkey function can execute in parallel, But when update function executing, getKey function need to wait till the execution completes. – Kajal Jun 08 '17 at 10:31
Yes, so what if the update function starts immediately after `while(isLocked);`? It doesn't ensure that `getKey()` won't run while update is taking place. It only ensures it isn't running at the beginning but doesn't make sure it isn't running throughout. You need to hold the lock throughout both write and read operations. – Persixty Jun 08 '17 at 10:33

How to implement such use case by Java Concurrency Lock

4 Answers4