Synchronized locks in java

Question

I've got multi threaded application(for example, servlet). Also I've got a common resource(file, ref or something). How can I synchronize its access? This is how I do:

    static Object lock = new Object();

    volatile int commonRes = 0;

    public void doSomething() {
        System.out.println("Before statement " + commonRes);

        synchronized (lock) {
            //some process
            commonRes++;
        }
        //Do something else
        System.out.println("After statement " + commonRes);
    }

I have two questions:

Is it good approach?
Should lock be volatile?

can't you simply lock the method that access the resource? using something like public synchronized void doSomething()? — LMG, May 13 '14 at 08:54
I've got no need to sync. whole method. Method could write into file or do some heavy process. — Tony, May 13 '14 at 08:58

score 4 · Answer 1 · answered May 13 '14 at 09:00

4

If both threads use the same object to reference the same file, there is no need for a separate static object. As your class is descended from Object, you can also use

synchronized(this) { ... }

or the shorthand form covering the entire method

public synchronized void doSomething() { ... }

The volatile is implied in synchronized accesses -- the locking primitives are aware they need to be thread-safe.

answered May 13 '14 at 09:00

Simon Richter

28,572
1
42
64

Locking on `this` is not that good. If a client decides to lock on a reference of your object, you will have problems. Also it doesn't resolves the problem of static and non-static methods synchronization. – Andrei May 13 '14 at 09:28
Check this answer to see a more detailed version of why it is bad (even if it is c#-ish): http://stackoverflow.com/questions/251391/why-is-lockthis-bad – Andrei May 13 '14 at 09:32

Andrei · Accepted Answer · 2014-05-13T09:31:36.160

Is it good approach?

Locking resources in multi-threaded environments is always good, but marking as volatile and synchronizing on the resource is redundant as far as I know since both accomplish almost the same thing in the posted scenario.

When I use synchronized blocks I prefer to lock on the class like this:

synchronized(MyClass.class) {
...
}

Why i do this:

both static and non-static methods will be blocked.
no other external object will be able to lock on your object and possibly create subtle bugs.

Still using synchronized blocks like you did is OK too in most scenarios (kinda like the same as what I prefer to do), but if you encounter problems (Performance, things you can't accomplish, starvation), I recommend using a structure like ReentrantLock.
It offers more flexibility and may offer some bonus performance. Also it can accomplish things a synchronized block can not.

Also something like ReadWriteLock or ReentrantReadWriteLock could serve you well in some scenarios so they are worth reading about too.

Should lock be volatile?

As far as I know there is no reason to. The locking objects are implied to be volatile like.

Synchronized locks in java

2 Answers2