synchronizing static methods in the same class

Question

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:

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?

score 5 · Answer 1 · answered Jun 22 '10 at 14:11

5

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.

answered Jun 22 '10 at 14:11

unbeli

29,501
5
55
57

Question asker here. The String value is related to the method name that I'm synchronizing so it won't appear elsewhere but I take your point, I'll use Objects instead. I know my solution works by the way, I'm not asking if it works but if there is a more elegant solution that doesn't involve creating objects solely for the purpose of locking? – CodeClimber Jun 22 '10 at 14:26
Creating objects for locking is ok. After all, this is the only thing you can lock on in Java, so you either create it or find a suitable existing object. – unbeli Jun 22 '10 at 14:30
Creating lock objects is a very common solution. – matt b Jun 22 '10 at 14:35
@dairemac: While `String` may be safe for static methods, you'll eventually get burned using String objects for synchronizing because Strings are interned; there is only one copy of a given `String`. – Powerlord Jun 22 '10 at 15:51
@R. Bemrose it's not about String as a class, but about string constants. You can safely use new String("whatever") if you like. – unbeli Jun 22 '10 at 17:16
I think `byte[] b = new byte[0]` has a smaller memory footprint – Bozho Jul 13 '10 at 21:18

score 1 · Answer 2 · answered Jun 22 '10 at 13:49

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.

answered Jun 22 '10 at 13:49

Dave

5,133
21
27

Tom Hawtin - tackline · Answer 3 · 2010-06-22T15:35:19.607

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.

score -2 · Answer 4 · answered Jun 22 '10 at 13:53

-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.

answered Jun 22 '10 at 13:53

Roman

64,384
92
238
332

1

this is actually worse than the author's version as one does not know what lock[0] is supposed to protect - it has no name. The only useful point is to use new Object() and not a "STRING" – unbeli Jun 22 '10 at 14:10
@Tom Hawtin - tackline: what's wrong with array? My answer may be wrong but your comment points nothing. Downvote you until better explanation appears. – Roman Jun 22 '10 at 14:22
What's wrong with the array?? It doesn't make any sense!! / So you've downvoted me for commenting on your answer? – Tom Hawtin - tackline Jun 22 '10 at 14:32
@Roman I explained you what's wrong with the array, don't be silly – unbeli Jun 22 '10 at 14:37
@Tom Hawtin - tackline, unbeli: so, the reason for 2 downvotes is that `lock[0]` doesn't have its own name and that's all? – Roman Jun 22 '10 at 14:55
2

@Roman yes. Anyone can write code that works. One must learn how to write code that is easy to read. – unbeli Jun 22 '10 at 15:13
I have to agree that using the array is a bad idea. Now doSomething's lock is tied to offset 0 of the array. "0" is quite meaning less when reading the code. And what happens if the array is changed, lock[0] = new String("something else"), now it's even more broken. – Steve Kuo Jun 22 '10 at 19:10

synchronizing static methods in the same class

4 Answers4