AutoLock

David Holmes David.Holmes at oracle.com
Mon Feb 28 18:05:35 PST 2011


(Re-send without cc's due to ongoing mailer issues)

Heinz,

 > I understand this is a contrived example, because we should never call
 > stop().  But I still like the way that the try-with-resource returns
 > the correct throwable  :-)

In my opinion it doesn't throw the "correct" throwable at all! The 
IllegalMonitorStateException would normally (not here because of stop()) 
indicate a significant programming error and so is much more significant 
in my view. Even worse for this example, ThreadDeath doesn't trigger 
printStackTrace in the default uncaught-exception handler, so you'd 
never even see you had this IllegalMonitorStateException lurking.

Just my 2c.

David

Dr Heinz M. Kabutz said the following on 03/01/11 05:07:
> Hi Joe, David,
> 
> thank you for humoring me regarding this issue.  I understand all of 
> your rationales for not allowing simple expressions.  I also agree with 
> Doug that we should not encourage people to write locking code, because 
> they will probably get it wrong.  This will probably be my last comment 
> on this and then I'll go back to my hole on the island of Crete and shut 
> up ;-)
> 
> Here is a piece of code that returns the correct exception when used 
> with my AutoLock try-with-resource approach, but returns an unexpected 
> exception when used with the traditional await approach, also used in 
> the BlockingQueues:
> 
> 
> import java.util.concurrent.locks.*;
> 
> public class StopTest {
>  private static Lock lock = new ReentrantLock();
>  private static Condition condition = lock.newCondition();
> 
>  private static void incorrectExceptionReturned() throws 
> InterruptedException {
>    lock.lock();
>    try {
>      condition.await();
>    } catch (InterruptedException e) {
>      condition.signal();
>      throw e;
>    } finally {
>      lock.unlock();
>    }
>  }
> 
>  private static void correctExceptionReturned() throws 
> InterruptedException {
>    try (AutoLock.lock(lock)) {
>      condition.await();
>    } catch(InterruptedException e) {
>      condition.signal();
>      throw e;
>    }
>  }
> 
>  public static void main(String[] args)
>      throws InterruptedException {
> 
>    Thread t1 = new Thread() {
>      public void run() {
>        try {
>          incorrectExceptionReturned();
>        } catch (Throwable t) {
>          System.out.println("incorrect exception: " + t);
>          t.printStackTrace(System.out);
>        }
>      }
>    };
>    Thread t2 = new Thread() {
>      public void run() {
>        try {
>          correctExceptionReturned();
>        } catch (Throwable t) {
>          System.out.println("correct exception: " + t);
>          t.printStackTrace(System.out);
>        }
>      }
>    };
>    t1.start();
>    t2.start();
>    Thread.sleep(100);
>    t1.stop();
>    Thread.sleep(100);
>    t2.stop();
>  }
> }
> 
> 
> I am calling the method stop().  Yes, I know it is deprecated.  I know 
> it should never be called.  I also know why :-)  However, until it is 
> removed from the JDK (never?), it will remain there and programmers will 
> be able to call it.  When I call stop() on a thread, I expect a 
> ThreadDeath.  However, if it is in the await() state, it will come back 
> from the await() without getting the lock again.  This means that we 
> will see the IllegalMonitorStateException, like so:
> 
> incorrect exception: java.lang.IllegalMonitorStateException
> java.lang.IllegalMonitorStateException
>    at 
> java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:155) 
> 
>    at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1258) 
> 
>    at 
> java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:459)
>    at 
> eu.javaspecialists.concurrent.StopTest.incorrectExceptionReturned(StopTest.java:17) 
> 
>    at eu.javaspecialists.concurrent.StopTest.access$000(StopTest.java:5)
>    at eu.javaspecialists.concurrent.StopTest$1.run(StopTest.java:36)
> correct exception: java.lang.ThreadDeath
> java.lang.ThreadDeath
>    at java.lang.Thread.stop(Thread.java:828)
>    at eu.javaspecialists.concurrent.StopTest.main(StopTest.java:58)
>    Suppressed: java.lang.IllegalMonitorStateException
>        at 
> java.util.concurrent.locks.ReentrantLock$Sync.tryRelease(ReentrantLock.java:155) 
> 
>        at 
> java.util.concurrent.locks.AbstractQueuedSynchronizer.release(AbstractQueuedSynchronizer.java:1258) 
> 
>        at 
> java.util.concurrent.locks.ReentrantLock.unlock(ReentrantLock.java:459)
>        at eu.javaspecialists.concurrent.AutoLock.close(AutoLock.java:18)
>        at 
> eu.javaspecialists.concurrent.StopTest.correctExceptionReturned(StopTest.java:24) 
> 
>        at 
> eu.javaspecialists.concurrent.StopTest.access$100(StopTest.java:5)
>        at eu.javaspecialists.concurrent.StopTest$2.run(StopTest.java:46)
> 
> I understand this is a contrived example, because we should never call 
> stop().  But I still like the way that the try-with-resource returns the 
> correct throwable :-)
> 
> Regards
> 
> Heinz



More information about the coin-dev mailing list