synchronizing static methods in the same class

Question

Hi. I have a class like this:

public class Utils {

    public static void doSomething() {
        // doSomething code
    }

    public static void doSomethingElse() {
        // doSomethingElse code
    }
}

I want the two methods to be synchronized but not synchronized against each other i.e. no two threads can process the doSomething() method at the same time, no two threads can process doSomethingElse() at the same time but it is OK for a thread to process the doSomething() method and another to process the doSomethingElse() method at the same time.

I've implemented something like this:

public class Utils {

    private static final String DO_SOMETHING_LOCK = "DO_SOMETHING_LOCK";
    private static final String DO_SOMETHING_ELSE_LOCK = "DO_SOMETHING_ELSE_LOCK";

    public static void doSomething() {
        synchronized(DO_SOMETHING_LOCK) {
            // doSomething code
        }
    }

    public static void doSomethingElse() {
        synchronized(DO_SOMETHING_ELSE_LOCK) {
        // doSomethingElse code
    }
}

I see the Scott Stanchfield response uses a similar approach here:

http://stackoverflow.com/questions/578904/how-do-synchronized-static-methods-work-in-java

but is this the best way to do this? It seems kind of clunky to me, creating two objects, only to be used for locking - is there a better way to do this?

Answer 1

Yes that's the easiest way. Although, the object that you lock on doesn't have to be anything special. You could just make them Integers with no values set.

Answer 2

Your solution seems correct. I usually see/use Object's instances as locks (instead of Strings), like:

public class Utils {

    private static final Object[] lock = new Object[] { new Object(), new Object() };

    public static void doSomething() {
        synchronized(lock[0]) {
            // doSomething code
        }
    }
    ...
}

The problem you're trying to solve is related to ThreadLocal class use cases.

Answer 3

Locking on interned Strings is a mistake. If you run FindBugs, I believe it will point this out for you. Create a new instance of an object for the lock - new Object() or new String("Something informative"). To see something useful in stack traces when it dead locks, use a custom class for the job.

private final Object lock = new Object() { };

or

private static final class MyLock { }
private final Object lock = new MyLock();

In fact, it is generally better to create a private internal object (objects are really small - do the maths) than expose the lock through a public interface.

Answer 4

Using strings is not a good idea, because the same string might appear somewhere else, it will be the same object and you can get dependencies where you don't what them. Just do

private static final Object DO_SOMETHING_LOCK = new Object();

The rest is ok.