[concurrency-interest]Condition thoughts

Joseph Bowbeer jozart@csi.com
Sun, 18 Aug 2002 13:39:50 -0700


Eric,

I don't have any comment on the gist of your message, unfortunately, but I
noticed a recurring pattern in the code snippets that raised a red flag:

  public synchronized void await() {
    try {
      lock.release();
      wait();
    } finally {
      lock.acquire();
    }
  }

I would code this as follows, moving the code that enters the protected
state *before* the try block:

  public synchronized void await() {
    lock.release(); // enter state
    try {
      wait();
    } finally {
      lock.acquire(); // leave state
    }
  }

This way, if there's a problem entering the state, the await() won't
compound the problem even further trying to leave a state that was never
entered.

Of course, acquire() is usually the method that enters the state and
release() is the one that leaves the state, but I don't think it matters to
try-finally if they are reversed:

  public synchronized void await() {
    lock.acquire(); // enter state
    try {
      work();
    } finally {
      lock.release(); // exit state
    }
  }

Using the try-finally in the way I've demonstrated is particularly important
when using locks that do *not* maintain ownership, such as the reader-lock
we've been kicking around.  For example, if lock.acquire() were moved inside
the try, then a failure in acquire() could release someone else's lock count
in the finally, thereby permitting a writer to acquire the resource even
though a reader is still active.

Note that lock.acquire() can fail if the thread has been interrupted.
(Moving acquire inside the try will teach 'em not to interrupt!)


By the way, both of the monitorEnter(Object[]) and monitorExit(Object[])
implementations call monitorEnter(null, Object[]).  I'm guessing that's a
typo.



----- Original Message -----
From: "Eric D Crahen" <crahen@cse.Buffalo.EDU>
To: <concurrency-interest@altair.cs.oswego.edu>
Sent: Friday, August 09, 2002 5:29 PM
Subject: [concurrency-interest]Condition thoughts


I was looking at the Condition class a little bit more today. I
implemented a simplified version (because I didn't want to write Clock
classes too) to expirement with it a little bit to get a better feel for
it; I attached it at the bottom of this message incase anyone want to
play with it. Anyway, these are the thoughts I was having about
the Condition interface.

One of the things I really liked about this version compared to the
previous version was that I felt it helped make things more expressive.
You don't need as many try { } finally { } blocks, so things a less
cluttered; and classes like,

class aClass { // pseduocode

  Condition cond = new Condition(this);

  public synchronized void aMethod() {

     // ...

     while(someConditionNotMet())
       cond.wait();

     // ...

  }

}

seem very clear as well (at least given my current understanding of it).

One of the things I found somewhat awkward about this is that is mixes in
objects that are interacting with the primitve language constructs. It may
just be my personal preference, but I just feel like it's missing some
kind of interaction with the Lock hierachy.

Its a shame that I can't use non-recursive locks, or use locks like
RecursiveLock whose acquire()s can be interrupted with a Condtion.
I know the general consensus seems to be recursive locks are better, but I
still think it would be good leave the choice to user. Doug Lea mentioned
that we can realize non-recursive locks through a Semaphore with the an
intial size of 1 - so it is possible to use them if you'd like. Please
don't think I'm trying to start a debate or anything, I'm just explaining
my thoughts.

Now, after Dave's explaination of how this Condition would work, and after
implementing something concrete to play with today I was looking for a way
to keep the thing I liked, and to resolve the thing I didn't like. This is
what I came up with:

If to accomplish this, something would be added that was allowed to do
unbalanced monitorenter/monitorexits; why not make a Lock wrapper for an
objects monitor. Something like this could be used,

public ObjectLock implements Lock { // pseduocode

  private static MonitorHelper helper = new MonitorHelper();
  private Object lock;

  public ObjectLock(Object lock) {
    this.lock = lock;
  }

  public void acquire()
    throws InterruptedException {

    helper.monitorEnter(lock);

  }

  public void release()
    throws InterruptedException {

    helper.monitorExit(lock);

  }

}

to provide a Lock interface to an objects monitor. The Condition class
then might be revised to work with a Lock instead of an Object,

class Condition { // pseduocode

  private Lock lock;

  public Condition(Lock lock) {
    this.lock = lock;
  }

  public synchronized void await() {
    try {
      lock.release();
      wait();
    } finally {
      lock.acquire();
    }
  }

  // ...
}

So now, the small example could be rewritten like this,

class aClass {

  Lock lock = new ObjectLock(this);
  Condition cond = new Condition(lock);

  public synchronized void aMethod() {

     // ...

     while(someConditionNotMet())
       cond.wait();

     // ...

  }

}

What I think is nice about this is this:

 You can use the synchronized keyword when its appropriate and avoid the
 excessive try {} finally {} block that you used to have to write when
 using Locks only.

 The ObjectLock fits into the Lock hierachy, and for me it just feels a
 little better. It fits better with OO design of the rest of the package.
 Again, this is just my opionion, but maybe some others also would like
this.

 The part I like the most is that now the Lock the Condition works with is
 interchangeable with other Locks. I could use a non-recusive lock by
 changing one line [lock = new Semaphore(1)] or, if I wanted to, I could
 use a lock with an interruptible acquire by changing one line [lock = new
 RecursiveLock()]

What do you (anyone) think?

This is the implementation I mentioned earlier. Basically, it creates a
class that will attempt monitorenter & monitorexit when certain methods
are called. Depending on your VM, it may or may not be allowed; it seems
to be an optional restriction. I've been using IBM's 1.3.0 VM today to
make this, and it did not complain. Sun's 1.4.0 VM does check to make sure
monitorenter & monitorexit are balanced in each method, so if you run this
on that VM you'll see an IllegalMonitorStateException. There might be an
option to turn that off, but I couldn't find it.

(It was all done with 100% Java by the way, I used a bytecode toolkit
thats part of the polymorph package on my web page to create the
MonitorHelper)

// The source is also at
// http://www.cse.buffalo.edu/~crahen/jsr166/Condition.java
----------------------------------------------------------------------------
-
// Condition.java begin

import java.lang.reflect.Method;
import java.lang.reflect.InvocationTargetException;

/**
 * @class Condition
 * @author Eric Crahen
 *
 * Condition implementation to expirement with
 */
public class Condition {

  private static MonitorHelper helper = new MonitorHelper();

  private Object sync = new Object();
  private Object[] param;

  public Condition(Object lock)
    throws InterruptedException {

    if(lock == null)
      throw new IllegalArgumentException();

    param = new Object[]{lock};

  }

  public void await() throws InterruptedException {

    synchronized(sync) {

      try {
        helper.monitorExit(param);
        sync.wait();
      } finally {
        helper.monitorEnter(param);
      }

    }

  }


  public void signal() {

    synchronized(sync) { sync.notify(); }

  }

  public void signalAll() {

    synchronized(sync) { sync.notifyAll(); }

  }

  static class MonitorHelper extends ClassLoader {

  /**
   * package java.util.concurrent;
   * public abstract class Monitor {
   *   protected Monitor();
   *   public static void enterMonitor(Object o);
   *   public static void exitMonitor(Object o);
   * }
   */
  private final static byte[] bytecode = new byte[] {
   (byte)0xca, (byte)0xfe, (byte)0xba, (byte)0xbe,    0x00, 0x03,
0x00, 0x2d,   0x00, 0x0d, 0x01, 0x00,   0x1c, 0x6a, 0x61, 0x76,
0x61, 0x2f, 0x75, 0x74,   0x69, 0x6c, 0x2f, 0x63,     0x6f, 0x6e, 0x63,
0x75,   0x72, 0x72, 0x65, 0x6e,   0x74, 0x2f, 0x4d, 0x6f,   0x6e,
0x69, 0x74, 0x6f,   0x72, 0x07, 0x00, 0x01,   0x01, 0x00, 0x10, 0x6a,
0x61, 0x76, 0x61, 0x2f,   0x6c, 0x61, 0x6e, 0x67,   0x2f, 0x4f, 0x62,
0x6a,   0x65, 0x63, 0x74, 0x07,   0x00, 0x03, 0x01, 0x00,   0x06,
0x3c, 0x69, 0x6e,   0x69, 0x74, 0x3e, 0x01,   0x00, 0x03, 0x28, 0x29,
0x56, 0x01, 0x00, 0x04,   0x43, 0x6f, 0x64, 0x65,   0x0c, 0x00,
0x05, 0x00,   0x06, 0x0a, 0x00, 0x04,   0x00, 0x08, 0x01, 0x00,
0x0c, 0x6d, 0x6f, 0x6e,   0x69, 0x74, 0x6f, 0x72,   0x45, 0x6e, 0x74,
0x65,   0x72, 0x01, 0x00, 0x15,   0x28, 0x4c, 0x6a, 0x61,   0x76,
0x61, 0x2f, 0x6c,   0x61, 0x6e, 0x67, 0x2f,   0x4f, 0x62, 0x6a, 0x65,
0x63, 0x74, 0x3b, 0x29,   0x56, 0x01, 0x00, 0x0b,   0x6d, 0x6f, 0x6e,
0x69,   0x74, 0x6f, 0x72, 0x45,   0x78, 0x69, 0x74, 0x04,   0x21,
0x00, 0x02, 0x00,   0x04, 0x00, 0x00, 0x00,   0x00, 0x00, 0x03, 0x00,
0x04, 0x00, 0x05, 0x00,   0x06, 0x00, 0x01, 0x00,   0x07, 0x00,
0x00, 0x00,   0x11, 0x00, 0x02, 0x00,   0x01, 0x00, 0x00, 0x00,
0x05, 0x2a, (byte)0xb7, 0x00,   0x09, (byte)0xb1, 0x00, 0x00,
0x00, 0x00, 0x00, 0x09,   0x00, 0x0a, 0x00, 0x0b,   0x00, 0x01, 0x00,
0x07,   0x00, 0x00, 0x00, 0x0f,   0x00, 0x01, 0x00, 0x01,   0x00,
0x00, 0x00, 0x03,   0x2a, (byte)0xc2, (byte)0xb1, 0x00,   0x00, 0x00,
0x00, 0x00,   0x09, 0x00, 0x0c, 0x00,   0x0b, 0x00, 0x01, 0x00,
0x07, 0x00, 0x00, 0x00,   0x0f, 0x00, 0x01, 0x00,   0x01, 0x00, 0x00,
0x00,   0x03, 0x2a, (byte)0xc3, (byte)0xb1,   0x00, 0x00, 0x00, 0x00,
0x00, 0x00
  };
  private static Method monitorEnter;
  private static Method monitorExit;

  MonitorHelper() {

    try {

      Class clazz = defineClass(null, bytecode, 0, bytecode.length);

      monitorEnter = clazz.getMethod("monitorEnter", new
Class[]{Object.class});
      monitorExit =  clazz.getMethod("monitorExit",  new
Class[]{Object.class});
    } catch(Throwable t) {

      t.printStackTrace();
      System.exit(0);

    }

  }

  void monitorEnter(Object[] o) {

    Error err = null;

    try {
      monitorEnter.invoke(null, o);
    } catch(InvocationTargetException e) {

      Throwable t = e.getTargetException();
      if(t instanceof RuntimeException)
        throw (RuntimeException)t;

      err = new Error(t);

    } catch(Exception e) {
      err = new Error(e);
    }

    if(err != null)
      throw err;

  }

  void monitorExit(Object[] o) {

    Error err = null;

    try {
      monitorEnter.invoke(null, o);
    } catch(InvocationTargetException e) {

      Throwable t = e.getTargetException();
      if(t instanceof RuntimeException)
        throw (RuntimeException)t;

      err = new Error(t);

    } catch(Exception e) {
      err = new Error(e);
    }

    if(err != null)
      throw err;

  }

  }

}

// Condition.java end



- Eric
http://www.cse.buffalo.edu/~crahen



_______________________________________________
Concurrency-interest mailing list
Concurrency-interest@altair.cs.oswego.edu
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest