CODEWITHSUNDEEP
javaconcurrencylockingdeadlock
Tapas Bose
Tapas Bose

Reputation: 29806

Locking in Highly Concurrent System

I have a class in a highly concurrent system. A method getResolvedClassName() of that class can produce deadlock. So I am designing it in the following way:

public class ClassUtils {

    private static ClassUtils classUtils;

    private transient Object object = new Object();

    private synchronized Object getObjectLock() {
        return object;
    }

    public void getResolvedClassName(Class<?> clazz) {
        synchronized (getObjectLock()) {
            //some job will be done here
        }       
    }

    public synchronized static ClassUtils getInstance() {
        if(classUtils == null) {
            classUtils = new ClassUtils();
        }
        return classUtils;
    }   
}

Am I doing it in a right way? Any information will be helpful to me.

Thanks.

Edit:

public class ClassUtils {

    private static final ClassUtils classUtils = new ClassUtils();
    private ReentrantLock lock = new ReentrantLock();

    public void getResolvedClassName(Class<?> clazz) {
        lock.lock();
        //some job will be done here
        lock.unlock();      
    }

    public static ClassUtils getInstance() {        
        return classUtils;
    }   
}

Upvotes: 0

Views: 258

Answers (3)

Jonathan
Jonathan

Reputation: 7604

A few things stand out:

  1. I don't think the transient keyword means what you think it means. That keyword has nothing to do with synchronization, and is only used when serializing a class. You might be confusing it with volatile. Incidentally, volatile is not needed here either.

  2. Lazy initialization of your singleton is probably unnecessary. Why don't you just do private static final ClassUtils classUtils = new ClassUtils();? Then your getInstance() method does not need to be synchronized and can just return classUtils; It is also thread safe. You should also always declare singleton instances as final.

  3. The whole situation with getObjectLock() is not needed. You can just synchronize on this (i.e. make getResolvedClassname into a synchronized method) and will be safer and cleaner.

You could also investigate the java.util.concurrent.Lock classes to see if there is something more suitable than synchronizing on an Object, which is nowadays considered poor form.

Upvotes: 3

Malcolm
Malcolm

Reputation: 41510

This question is really a little vague, I don't see the purpose of using a singleton and why synchronization is required to do some job. If it doesn't access mutable state, it doesn't need synchronization. I can only say that three locks (ClassUtils.class, ClassUtils instance, and object) are almost certainly adding unnecessary complexity. Also, as Justin noted, you should make object final, then you won't need synchronization to access it.

Upvotes: 3

justin
justin

Reputation: 104698

Your problem is a bit general. However, you can consider initializing that value as immutable. Many immutable, initialized values are threadsafe, and require no locking.

Upvotes: 0

Related Questions