CODEWITHSUNDEEP
javamultithreadingsynchronizationlocking
user1589188
user1589188

Reputation: 5736

JAVA synchronising using random string

I have a critical process that I have to make sure at any one time there cannot be two equivalent MyObject being processed (can be different instances but logically equal). The following code demonstrates the idea:

public class MyClass {
    public static ConcurrentMap<MyObject, String> concurrentMap = new ConcurrentHashMap<>();
    public void process(MyObject myObject) {
        String id = UUID.randomUUID().toString();
        String existingId = concurrentMap.putIfAbsent(myObject, id);
        synchronized (id) {
            if (existingId == null) {  // no others, start working right away
                // do work
            } else {  // an equivalent myObject is under processing, wait on it
                synchronized (existingId) {
                    // finally can start doing work
                }
            }
        }
    }
}

The code above works with the help of synchronized on a random string. But the issues with this code are

  1. Every time it creates a new random id but is not used if an existing id has been linked to an equivalent MyObject. The sole purpose of such id is to act as a unique lock to be discovered by the other thread. Thinking if this can be replaced by some actual lock object?

  2. There is no way in this code to know when should MyObject be removed from the concurrentMap, although this does not affect the result, having the concurrentMap keeps growing may not be good. Thinking if something like a counter of locks can be used here?

Thank you

Upvotes: 1

Views: 105

Answers (2)

Mikita Harbacheuski
Mikita Harbacheuski

Reputation: 2253

I think this library is what you are looking for. Particularly StripedKeyLockManager and CountingLock classes which address your questions. You can either use the library in your project or adjust its source code for your needs. Guava also provides similar functionality via Stripped class.

Upvotes: 0

markspace
markspace

Reputation: 11030

I don't think I really understand the use case for this idea, and that's a big red flag. But it seems to me that all of this code is unnecessary. If the only idea here is to obtains a unique lock for myObject, then you already have that: it's myObject.

public class MyClass {

    public void process(MyObject myObject) {
       synchronized (myObject) {
           // finally can start doing work
       }
    }
}

The rest of the code is just dead weight.

However this is still fraught. Since you are relying on an external procedure to synchronize your object, any other bit of code with a reference to myObject can do whatever they like and you have no control over it. It's a really weak form of synchronization. It can work, if everyone in the code base understands the need to synchronize on their MyObject, but this could be tough to achieve in practice.

Upvotes: 2

Related Questions