Locking scheme

[Mr. C Lamont Gilbert]

I have a situation where lots of other classes will be manipulating a Collection. I am wondering which of these two locking schemes you thing is preferred in an OO sense.

Style 1

class Resource {
   private list;
   private Object listLock = new Object();
   void add(Object o){
       list.add(o);
   }
   Object getLockObj(){
      return this.listLock;
   }
}
 
class User {
   public void doit(){
       Resource r = getResource();
       synchronized(r.getLockObj()){
           r.add(new Object());
       }
   }
}

Style 2

class Resource {
   private list;
   private Object listLock = new Object();
   private boolean locked;
   void add(Object o){
       list.add(o);
   }
   void lock(){
      synchronized(this.listLock){
         while(this.locked == true){
             this.listLock.wait();
         }
         this.locked = true;
      }
   }
   void unlock(){
       synchronized(this.listLock){
           this.locked = false;
           this.listLock.notify();
        }
   }
}
 
class User {
   public void doit(){
       Resource r = getResource();
       r.lock();
       r.add(new Object());
       r.unlock();
   }
}

I prefer the 2nd approach but the problem is it does not care who locked it. It will let anybody unlock it. plus you can forget to unlock it. I can record which thread locked it and assert if its not the same as the one that unlocked it. However, the first approach seems not so OO in design, but its simple and foolproof AFAICT.

Are there other resource locking schemes?

[Ilja Preuss]

I must be missing something: why don't you simply make add() synchronized???

[Mr. C Lamont Gilbert]

Originally posted by Ilja Preuss:
I must be missing something: why don't you simply make add() synchronized???

I don't use synchronized methods because I don't like to expose the monitor. That is why I have the two alternate methods. Of the two, style 1 is basically like what you suggest but I don't lock on 'this' I lock on the internal object created for locking.

Furthermore, there is a class heirachy. Some methods are in the super class and some are in the sub class. Both share the same resource and so what ever lock I use in the super class I need to use in the sub class as well.

Style one seems so un-OO, yet its so easy. Tbh, I started with style 2 before I realized I could do style 1
[ September 25, 2005: Message edited by: Mr. C Lamont Gilbert ]

[Stan James]

I think the very first sentence of the original post gives me the most trouble. Can you make that go away?

[Ilja Preuss]

Originally posted by Mr. C Lamont Gilbert:
I don't use synchronized methods because I don't like to expose the monitor.

Then why not do

class Resource {
   private list;
   private Object listLock = new Object();
   void add(Object o){
      synchronized(listLock) {
         list.add(o);
      }
   }
}

[Mr. C Lamont Gilbert]

Originally posted by Ilja Preuss:

class Resource {
   private list;
   private Object listLock = new Object();
   void add(Object o){
      synchronized(listLock) {
         list.add(o);
      }
   }
}

because the subclass does not have access to listlock in your example. The key to my situation is the subclasses and super classes all have things they need to do and they need to lock while doing them.

For you Stan lets say two (2) other classes.

[Stan James]

How does this make you feel?

class Resource {
   private list;
   private Object listLock = new Object();
   void add(Object o){
      synchronized(listLock) {
         if (preAdd(o)) {
            list.add(o);
            postAdd(o);
         }
      }
   }
   boolean preAdd( Object o )
   {
      return true;
   }
   etc ...
}

Now subclasses have well defined extension points. I have to admit it makes me feel pretty queasy. We did lots of this in PowerBuilder in the previous millenium, almost none in Java lately.

[Mr. C Lamont Gilbert]

class Parent {
   private Object syncObject = new Object();
   final Object getSyncObject(){
      return syncObject;
   }
 
   public final void doThing(){
      synchronized(getSyncObject()){
           //do stuff
      }
   }
}
class Junior extends parent {
   public final void doOtherThing(){
      synchronized(getSyncObject()){
         //do other stuff
      }
   }
}

This is what I am doing. The parent holds a resource that must be locked during use so to speak. The decendent locks it before he uses it. The other technique I used is listed in the OP and involves calling lock and unlock methods before and after the "//do stuff" parts.

[Mr. C Lamont Gilbert]

I just thought of another technique. Lockers can push into a stack. If you are at the top of the stack, you have the lock. If you are not, you must wait until you are before you consider yourself as having the lock. So when a locker is done with the lock, he can remove himself from the stack, and notify the other waiters. they will all wake up, or perhaps you sleep on your own lock thingy you put in the stack. that way just one can be notified.

This would work and is a more OO approach I think. But it sure is much more complicated and also has a problem with the unlocker possibly not being able to unlock since the stack is locked by another thread trying to lock. Which I can use another lock and sleep/notify to get around but it sure is getting complex. I think maybe ill just stick to passing out a ref to the lock object.

[Ilja Preuss]

In your original post it looked like the lock-getter was used by clients of the class, which I think breaks encapsulation in a nasty way. Using it in subclasses for additional methods that need to be synchronized looks much better to me.

[Stan James]

Do you need a syncobject? Can't subclass and superclass all sync on "this" ?

[Mr. C Lamont Gilbert]

Originally posted by Ilja Preuss:
In your original post it looked like the lock-getter was used by clients of the class, which I think breaks encapsulation in a nasty way. Using it in subclasses for additional methods that need to be synchronized looks much better to me.

Its not a violation of encapsulation to expose something if its just to a child class? I thought it would be and that was why the preference for composition.

Originally posted by Stan James :

Do you need a syncobject? Can't subclass and superclass all sync on "this" ?

Are you messin' with me? Everyone here knows its my petpeeve to not lock on an exposed 'this' object since it would violate encapsulation.
[ September 29, 2005: Message edited by: Mr. C Lamont Gilbert ]

[Stan James]

Step me through how "synchronized (this)" violates encapsulation. I'm not gettin it.

[Mr. C Lamont Gilbert]

class AClass {
    public Object lockObject = new Object();
    public void doThing(){
        synchronized(lockObject){
            //do something
        }
    }
}

Do you agree that this is bad because lockObject is publicly visible?

"this" is not encapsulated, therefore using it as a lock object puts you in a precarious position. Look at Thread.join and see how it is locking on "this" and imagine what happens if someone else is locking on Thread.this!

To be clear, join will be stuck waiting on the monitor. Furthermore, can Sun fix this goof? Nope, because the behavior is exposed and it could effect the behavior of existing programs indavertently using this behavior.
[ September 30, 2005: Message edited by: Mr. C Lamont Gilbert ]

[Stan James]

Interesting. So I like your solution of the 28th.

I think I've been living with an invalid assumption (or a convention in my own code) that everybody who locks on a monitor does so for the same reason and it's correct for them to wait in line. I have done nothing to prevent some rogue thread from grabbing a monitor for some unanticipated reason and clogging up the works. And so far it hasn't bitten me. I'm going to have to devote some more time to the risks & benefits here.
[ October 01, 2005: Message edited by: Stan James ]