Can a shared resource safely be used as the lock for a synchrnized block?

I often find myself with code like

private static final MyType sharedResource = MyType();
private static final Object lock = new Object();
...
synchronized(lock)
{
    //do stuff with sharedResource
}

Is this really neccessary or could sharedResource be used as the lock itself like

private static final MyType sharedResource = MyType();
...
开发者_运维技巧synchronized(sharedResource)
{
    //do stuff with sharedResource
}

Note: synchronized blocks shown in the examples would live within methods doing work and are not methods themselves or synchronized methods.

EDIT: A very good point has been pointed out in some of the answers that if we are dealing with multiple shared resources that the first "Object" technique is far safer.

WARNING The fact that sharedResource is static is important! If it is static then synchronized methods or synchronized blocks locking on this won't work. The lock object must also be static.

Pros and cons of both

First option pros: Allows locking on concept, not on Objects. If you had to lock on multiple resources for a single action (which is usually not advised, but sometimes necessary) you could do this with much less worry about race conditions.

cons: The object could still be modified, so you need to ensure access to the object is restricted to methods that abide by the external lock.

Second option pros: Lock on the object should prevent others from modifying it (although you should double check the exact symantics.) EDIT: Has the same con as above - if the methods aren't synchronized internally, you could still run into methods that don't abide by the contract.

cons: You block access to all methods, even those unrelated to what you're trying to operate, which could cause slowdowns and possibly deadlock. You could easily make the case, however, that if that's the case your Object is doing too much work and should be broken up.

EDIT: Allow me to clarify part 2 here (the case to break up MyType into MyFoo and MyBar is open for debate...)

class MyType {
   Foo foo;
   Bar bar;

   void doFoo() { foo.do(); }
   void doBar() { bar.do(); }
}

class MyActions {
    MyType thing;

    void lotsOfFoo() {
        // blocks bar :-(
        synchronized(thing) { thing.doFoo(); } 
    }

    void lotsOfBar() {
        // blocks foo :-(
        synchronized(thing) { thing.doBar(); } 
    }

}

Personally, I use option 1 much more often (that's why I'm unsure about that part in option 2).

The only problem I see, with using sharedResource as a lock if MyType has methods defined as synchronized itself. In this case, some strange behaviour my occur that was not intended by the developers of MyType. (See glowcoder's comment for an example.)

Otherwise it should be fine, however if you do only need one lock anyway, then just synchronize on this instead of introducing an additional dummy object.

Btw, did you itentionally make your lock object static? Because this means that all instance lock on the same monitor, i.e. they can block each other, which may not bee the behaviour you intended.

Yes, you can absolutely do that.

In fact it improves clarity and reduces clutter.

If I remembered right, synchronized is a monitor, it grants that only one thread can access that object at any given time in the same JVM. So I think you are only accessing shardResouce, synchronizing on the shardResouce is sufficient.

Personally, I don't usually synchronize on an arbitrary lock. That is, I'd usually do something along the lines of your second approach:

private static final MyType sharedResource = MyType();
...
synchronized(sharedResource) {
    //do stuff with sharedResource
}

Of course, before running synchronized code, the lock for the target object must be obtained. Often, I take the locking "down" a step further, so to speak. Meaning, I would synchronize on a method within "sharedResource." As in:

public class MyType {

    public void synchronized doWork() {

    }
}

But when it comes to this kind of thing, it's hard to make generalizations because there are exceptions to every rule. In the end, your overall architecture requirements will dictate where your locking should occur. For me, most often, I find myself synchronizing the top-level method that accesses the shared resources and it is thereby more rare to lock on an object who does nothing more than serve as a lock.

EDIT: minor grammar fixes