Java synchronization depending on method parameter

Question

how can I provide synchronization upon method parameter values?

All method calls using the 'same' parameter value A should be synchronized. A method call with a different parameter value e.g. B can access, even when calls with A are already waiting. The next concurrent call for B must wait also for the first B to be released.

My use case: I want to synchronize the access to JPA entities on ID level but want to avoid pessimistic locking because I need kind of a queue. The 'key' for locking is intended to be the entity ID - which is in fact of the type Java Long.

protected void entityLockedAccess(SomeEntity myEntity) {
    //getId() returns different Long objects so the lock does not work
    synchronized (myEntity.getId()) {
        //the critical section ...
    }
}

I read about lock objects but I am not sure how they would suit in my case. On the top level I want to manage a specific REST call to my application which executes critical code.

Thanks, Chris

Macias

Why do you need to synchronize access. "I need kind of a queue" doesn't really explain it. Unless you have a really good reason I'd recommend against reimplementing entity locking in your own convoluted way. – Putdown

There is a really high chance to run into either pessimistic or optimistic locking exceptions because there are a lot of concurrent calls to the same entity, each modifying a collection in the enttiy. Any suggestions on how to solve this? – Macias

Well, something like CQRS might be applicable here. The main question is why are there so many concurrent calls to the same entities (enough to make optimistic locking a non-viable alternative). You don't want to start creating your own "solutions" when you're dealing with a database and aren't very experienced with locking. – Putdown

Yes - I don't want to start my own solution for the problem. But what could a pre-built solution be? It is not only the entity access should be locked but there are also other operations which are in the critical code section. – Macias

You're looking for a quick fix for something I suspect to be a design issue. – Putdown

This question is answered here Java synchronized method around parameter value. – Nell

This question is answered here Java Synchronized Method Around Parameter Value. – Nell

This question is answered here Java Synchronized Method Around Parameter Value. – Nell

This question is answered here Java Synchronized Method Around Parameter Value. – Nell

Answer 1

As far as I understood you basically want a different, unique lock for each of your SomeEntity IDs.

You could realize this with a Map<Integer, Object>.

You simply map each ID to an object. Should there already be an object, you reuse it. This could look something like this:

static Map<Integer, Object> locks = new ConcurrentHashMap<>();

public static void main(String[] args)
{
    int i1 = 1;
    int i2 = 2;

    foo(i1);
    foo(i1);
    foo(i2);
}

public static void foo(int o)
{
    synchronized (locks.computeIfAbsent(o, k -> new Object()))
    {
        // computation
    }
}

This will create 2 lock objects in the map as the object for i1 is reused in the second foo(i1) call.

Americana

Thanks - Is "locks.computeIfAbsent" in the synchronized statement already synchronized? Any chance that I also run into concurrency problems here? – Macias

The implementation of computeIfAbsent for ConcurrentHashMaps is quite complex. It is guaranteed to happen atomic and the update itself that happens when a key is not found is synchronized. In any realistic scenarios this here should therefore never run into trouble. You can theoretically run into issues when a value already exists and is not in the first bucket of the Map as far as I am aware. Compare this question for more sophisticated input here. – Americana

Maybe to elaborate a bit further: What could in theory happen is that two objects are created and put in the map, with the second overriding the first. It is however still guaranteed that both calls return the second (and current) value as the operation is atomic. Which means you won't run into any concurrency problems but you could create an unneccessary object (which should really not be a performance concern imho) – Americana

Thanks! I think I will go with this approach although there is a litte downside because the Map will increase in size over time. – Macias

To avoid the map increasing over time, is it safe to remove the lock as the last statement inside the synchronized block? Seems to be ok at first glance, but trying to figure out if I'm missing something. – Unicycle

Can we use weakhashmap ? – Successful

Answer 2

The problem is that you simply should not synchronize on values (for example strings, or Integer objects).

Meaning: you would need to define some special EntityId class here, and of course, all "data" that uses the same ID would somehow need to be using the same EntityId object then.

Gadson

Answer 3

Objects which are pooled and potentially reused should not be used for synchronization. If they are, it can cause unrelated threads to deadlock with unhelpful stacktraces.

Specifically, String literals, and boxed primitives such as Integers should NOT be used as lock objects because they are pooled and reused.

The story is even worse for Boolean objects because there are only two instances of Boolean, Boolean.TRUE and Boolean.FALSE and every class that uses a Boolean will be referring to one of the two.

I read about lock objects but I am not sure how they would suit in my case. On the top level I want to manage a specific REST call to my application which executes critical code.

You DB will take care for concurrent writes and other transactional issues. All you need to do is use Transactions.

I would also recommend you to go through the classical problems (DIRTY READs NON Repeatable reads). You can also use Optimistic Locking for

Buckinghamshire

Answer 4

    private static final Set<Integer> lockedIds = new HashSet<>();

    private void lock(Integer id) throws InterruptedException {
        synchronized (lockedIds) {
            while (!lockedIds.add(id)) {
                lockedIds.wait();
            }
        }
    }

    private void unlock(Integer id) {
        synchronized (lockedIds) {
            lockedIds.remove(id);
            lockedIds.notifyAll();
        }
    }

    public void entityLockedAccess(SomeEntity myEntity) throws InterruptedException {
        try {
            lock(myEntity.getId());

            //Put your code here.
            //For different ids it is executed in parallel.
            //For equal ids it is executed synchronously.

        } finally {
            unlock(myEntity.getId());
        }
    }

• id can be not only an 'Integer' but any class with correctly overridden 'equals' and 'hashCode' methods.
• try-finally - is very important - you must guarantee to unlock waiting threads after your operation even if your operation threw exception.
• It will not work if your back-end is distributed across multiple servers/JVMs.

Nell

Answer 5

Just use this class: (and the map will NOT increase in size over time)

import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;

public class SameKeySynchronizer<T> {
    
    private final ConcurrentHashMap<T, Object> sameKeyTasks = new ConcurrentHashMap<>();

    public void serializeSameKeys(T key, Consumer<T> keyConsumer) {
        // This map will never be filled (because function returns null), it is only used for synchronization purposes for the same key
        sameKeyTasks.computeIfAbsent(key, inputArgumentKey -> acceptReturningNull(inputArgumentKey, keyConsumer));
    }

    private Object acceptReturningNull(T inputArgumentKey, Consumer<T> keyConsumer) {
        keyConsumer.accept(inputArgumentKey);
        return null;
    }
}

Like in this test:

import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

class SameKeySynchronizerTest {

    private static final boolean SHOW_FAILING_TEST = false;

    @Test
    void sameKeysAreNotExecutedParallel() throws InterruptedException {

        TestService testService = new TestService();

        TestServiceThread testServiceThread1 = new TestServiceThread(testService, "a");
        TestServiceThread testServiceThread2 = new TestServiceThread(testService, "a");

        testServiceThread1.start();
        testServiceThread2.start();

        testServiceThread1.join();
        testServiceThread2.join();

        Assertions.assertFalse(testService.sameKeyInProgressSimultaneously);
    }

    @Test
    void differentKeysAreExecutedParallel() throws InterruptedException {

        TestService testService = new TestService();

        TestServiceThread testServiceThread1 = new TestServiceThread(testService, "a");
        TestServiceThread testServiceThread2 = new TestServiceThread(testService, "b");

        testServiceThread1.start();
        testServiceThread2.start();

        testServiceThread1.join();
        testServiceThread2.join();

        Assertions.assertFalse(testService.sameKeyInProgressSimultaneously);
        Assertions.assertTrue(testService.differentKeysInProgressSimultaneously);
    }

    private class TestServiceThread extends Thread {
        TestService testService;
        String key;

        TestServiceThread(TestService testService, String key) {
            this.testService = testService;
            this.key = key;
        }

        @Override
        public void run() {
            testService.process(key);
        }
    }

    private class TestService {

        private final SameKeySynchronizer<String> sameKeySynchronizer = new SameKeySynchronizer<>();

        private Set<String> keysInProgress = ConcurrentHashMap.newKeySet();
        private boolean sameKeyInProgressSimultaneously = false;
        private boolean differentKeysInProgressSimultaneously = false;

        void process(String key) {
            if (SHOW_FAILING_TEST) {
                processInternal(key);
            } else {
                sameKeySynchronizer.serializeSameKeys(key, inputArgumentKey -> processInternal(inputArgumentKey));
            }
        }

        @SuppressWarnings("MagicNumber")
        private void processInternal(String key) {
            try {
                boolean keyInProgress = !keysInProgress.add(key);
                if (keyInProgress) {
                    sameKeyInProgressSimultaneously = true;
                }
                try {
                    int sleepTimeInMillis = 100;
                    for (long elapsedTimeInMillis = 0; elapsedTimeInMillis < 1000; elapsedTimeInMillis += sleepTimeInMillis) {
                        Thread.sleep(sleepTimeInMillis);
                        if (keysInProgress.size() > 1) {
                            differentKeysInProgressSimultaneously = true;
                        }
                    }
                } catch (InterruptedException e) {
                    throw new IllegalStateException(e);
                }
            } finally {
                keysInProgress.remove(key);
            }
        }
    }
}

Exclude