Interrupted exception chaining

Discussion in 'Java' started by raphfrk@gmail.com, Sep 25, 2012.

  1. Guest

    Is there a recommended way of "chaining" interrupted exceptions?

    This is to implement a method call that doesn't throw an interrupted exception, but which calls a method which can be interrupted.

    public void uninterruptableWait(Object c) {
    boolean done = false;
    boolean interrupted = false;
    synchronized (c) {
    while (!done) {
    try {
    c.wait();
    done = true;
    } catch (InterrupedException ie) {
    interrupted = true;
    }
    }
    }
    if (interrupted) {
    Thread.currentThread().interrupt();
    }
    }

    If that interrupt was unexpected, and causes a stack trace, then it would be nice if it could include the details from the thrown exception.

    Is there a better way to do the above?
     
    , Sep 25, 2012
    #1
    1. Advertising

  2. Eric Sosman Guest

    On 9/25/2012 6:25 AM, wrote:
    > Is there a recommended way of "chaining" interrupted exceptions?
    >
    > This is to implement a method call that doesn't throw an interrupted exception, but which calls a method which can be interrupted.
    >
    > public void uninterruptableWait(Object c) {


    (Aside: This could probably be a `static' method.)

    > boolean done = false;
    > boolean interrupted = false;
    > synchronized (c) {
    > while (!done) {
    > try {
    > c.wait();
    > done = true;
    > } catch (InterrupedException ie) {


    (Aside: `InterrupttttttttttttttttttttedException'.)

    > interrupted = true;
    > }
    > }
    > }
    > if (interrupted) {
    > Thread.currentThread().interrupt();
    > }
    > }
    >
    > If that interrupt was unexpected, and causes a stack trace, then it would be nice if it could include the details from the thrown exception.
    >
    > Is there a better way to do the above?


    It seems you're trying to have things both ways: You "expect"
    the interrupt by catching it, but then want to consider it as
    "unexpected" anyhow. Also, you want to inform your caller but
    have chosen not to throw an informative exception.

    Since an exception is just an object, I suppose you *could*
    have the method return it, or return `null' if there was none:

    /**
    * Waits for notify() or notifyAll() on an object, in
    * spite of interruptions.
    * @param c The object to wait for.
    * @return {@code null} if no interrupts occurred while
    * waiting, or one of the {@code InterruptedException}s
    * if one or more were thrown.
    */
    public static InterruptedException
    uninterruptableWait(Object c) {
    InterruptedException ex = null;
    for (;;) {
    try {
    c.wait();
    return ex;
    } catch (InterruptedException ie) {
    ex = ie;
    }
    }
    }

    This looks vile to me, though. I think your difficulty is
    self-inflicted, and should be solved by rethinking your design.

    --
    Eric Sosman
    d
     
    Eric Sosman, Sep 25, 2012
    #2
    1. Advertising

  3. markspace Guest

    On 9/25/2012 3:25 AM, wrote:
    > Is there a recommended way of "chaining" interrupted exceptions?



    I mostly agree with Eric. You shouldn't suppress interrupts unless you
    must. If for example you are implementing a Runnable, you can't throw
    InterruptedException but must deal with it somehow. Then catching an
    interrupted exception is reasonable.

    In this case however it's 100% your code and you could throw
    InterruptedException. That would defeat the purpose of the code you
    show, of course, but then that is also our point: what you are doing is
    rather questionable.

    There's no standard pattern for this because it's not done. I'd say the
    closest we have to to use a finally block to make certain the interrupt
    flag is restored.


    /** A questionable method: wait without throwing InterruptedException. */
    public void uninterruptableWait(Object c) {
    boolean interrupted = false;
    try {
    synchronized(c) {
    for(;;) {
    try {
    c.wait();
    return;
    } catch(InterruptedExcpetion ex ) {
    interrupted = true;
    }
    }
    }
    } finally {
    if(interrupted == true ) Thread.currentThread().interrupt();
    }
    }

    > If that interrupt was unexpected, and causes a stack trace, then it
    > would be nice if it could include the details from the thrown
    > exception.



    If you really need the details of the exception but can't throw from
    your local use site, I'd say wrapping the exception in a
    RuntimeExcpetion and throwing that is best. (Exception don't ever
    throwing RuntimeException; subclass it and throw your specific exception.)
     
    markspace, Sep 25, 2012
    #3
  4. Daniel Pitts Guest

    On 9/25/12 3:25 AM, wrote:
    > Is there a recommended way of "chaining" interrupted exceptions?
    >
    > This is to implement a method call that doesn't throw an interrupted exception, but which calls a method which can be interrupted.
    >
    > public void uninterruptableWait(Object c) {
    > boolean done = false;
    > boolean interrupted = false;
    > synchronized (c) {
    > while (!done) {
    > try {
    > c.wait();
    > done = true;
    > } catch (InterrupedException ie) {
    > interrupted = true;
    > }
    > }
    > }
    > if (interrupted) {
    > Thread.currentThread().interrupt();
    > }
    > }
    >
    > If that interrupt was unexpected, and causes a stack trace, then it would be nice if it could include the details from the thrown exception.
    >
    > Is there a better way to do the above?


    To take a pattern from Spring Binding (which may have taken it from
    elsewhere), you can keep a list of the caught exceptions, and then at
    the end of your method if that list is not empty, throw a new exception
    which contains the list.

    BTW, the stack-trace will only be on the c.wait() line, since that is
    where the interrupted exception will be thrown. This does not help you
    know who interrupted you unexpectedly.

    Do you really have a use-case for uninterruptableWait? Perhaps you
    should instead have a different approach to interrupting that thread.
    An interrupt is often a result of a user-action, and ignoring it will
    make users mad. It may also be the case that the interrupt was caused
    because something else failed, and waiting no longer is useful.

    I'd be curious to read your use case. I've written similar code in the
    past, and have since realized it was misguided.

    Good luck,
    Daniel.
     
    Daniel Pitts, Sep 25, 2012
    #4
  5. Ivan Ryan Guest

    On Sep 25, 4:33 pm, markspace <-@.> wrote:
    > On 9/25/2012 3:25 AM, wrote:
    >
    > > Is there a recommended way of "chaining" interrupted exceptions?

    >
    > I mostly agree with Eric.  You shouldn't suppress interrupts unless you
    > must.


    Well, the idea is to be able to have a method where the caller doesn't
    have to worry about handling interrupts.

    > If you really need the details of the exception but can't throw from
    > your local use site, I'd say wrapping the exception in a
    > RuntimeExcpetion and throwing that is best.  (Exception don't ever
    > throwing RuntimeException; subclass it and throw your specific exception.)


    It was to implement an interface.
    https://github.com/SpoutDev/SpoutAP...a/org/spout/api/util/concurrent/SpinLock.java
     
    Ivan Ryan, Sep 25, 2012
    #5
  6. Eric Sosman Guest

    On 9/25/2012 12:28 PM, Ivan Ryan wrote:
    > On Sep 25, 4:33 pm, markspace <-@.> wrote:
    >> On 9/25/2012 3:25 AM, wrote:
    >>
    >>> Is there a recommended way of "chaining" interrupted exceptions?

    >>
    >> I mostly agree with Eric. You shouldn't suppress interrupts unless you
    >> must.

    >
    > Well, the idea is to be able to have a method where the caller doesn't
    > have to worry about handling interrupts.


    Fine: The called method handles its own interrupts, and
    that's that. But then, the O.P. wants the information about
    the interrupt to propagate back to the caller -- the very same
    caller that "doesn't have to worry about handling interrupts!"

    "Pick a card, any card, don't show it to me. Okay, what
    card is it?"

    --
    Eric Sosman
    d
     
    Eric Sosman, Sep 25, 2012
    #6
  7. markspace Guest

    On 9/25/2012 9:28 AM, Ivan Ryan wrote:

    > It was to implement an interface.
    > https://github.com/SpoutDev/SpoutAP...a/org/spout/api/util/concurrent/SpinLock.java



    This looks 100% bogus to me. First, the JVM will insert spin-locks
    where needed in your code automatically. No need to try to build your
    own. The first rule of programming is "Don't re-invent the wheel."
    (The 0th rule is "Get a deposit with all contracts.")

    Second this is exactly the sort of situation where hiding an
    InterrruptedException will get you into trouble. Better to just
    re-throw and let the calling code deal with the exception (if desired).

    But even better to just delete this class and use
    java.util.concurrent.locks.ReetrantLock or something similar from the
    Java API.

    Plus, existing locks are more efficient that you might guess. I don't
    think an AtomicBoolean is going to beat them. Look into
    AbstractQueuedSynchronizer or LockSupport if you really must implement
    your own subclass of Lock.

    <http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/concurrent/locks/ReentrantLock.java#111>
     
    markspace, Sep 25, 2012
    #7
  8. Jan Burse Guest

    Hi,

    Daniel Pitts schrieb:
    > Do you really have a use-case for uninterruptableWait? Perhaps you
    > should instead have a different approach to interrupting that thread. An
    > interrupt is often a result of a user-action, and ignoring it will make
    > users mad. It may also be the case that the interrupt was caused
    > because something else failed, and waiting no longer is useful.


    Since he calls Thread.currentThread().interrupt() the next
    wait() will throw an InterruptedException. So it is not
    really ignored. It will be only ignored if all of the code
    uses uninterruptableWait(), and nowhere Thread.interrupted()
    is queried.

    wrote:
    > Is there a better way to do the above?


    But the biggest flaw I see in the original code, is that
    done=true is not called in the exception handler, so it will
    not break out of the code. Actually it will inflict a new
    InterruptedException by the wait() and so on.

    (Erics and marks code have a similar flaw, the flaw there
    is that wait() is again called, so code could block)

    Probably the code is only working, since wait()s are allowed
    to be spurious. But if this is not happening the CPU will
    burn, burn, burn, ...

    Bye
     
    Jan Burse, Sep 25, 2012
    #8
  9. markspace Guest

    On 9/25/2012 1:26 PM, Jan Burse wrote:

    >
    > (Erics and marks code have a similar flaw, the flaw there
    > is that wait() is again called, so code could block)



    That's actually a fair point. "Doesn't throw" isn't the same as
    "ignore." However, ReentrantLock is probably still better than the
    following code.


    /** A questionable method: wait without throwing InterruptedException. */
    public void uninterruptableWait(Object c) {
    boolean interrupted = false;
    try {
    synchronized(c) {
    try {
    c.wait();
    return;
    } catch(InterruptedExcpetion ex ) {
    interrupted = true;
    }
    }
    } finally {
    if(interrupted == true ) Thread.currentThread().interrupt();
    }
    }
     
    markspace, Sep 25, 2012
    #9
  10. Jan Burse Guest

    Hi,

    Anyway the code that will notify c should also set some state.
    If it doesn't we cannot distinguish between a spurious wait (*) and
    a wait by a notify. So code must use a loop, and to not confuse
    the done flag and loop breaking, we can just shift the try/catch
    outside of the loop, and even outside of the synchronized to
    minimize the monitor region:

    public boolean done; /* should be set by the thread that
    notifies the lock */
    public Object lock = new Object();

    public void uninterruptedWait() {
    try {
    synchronized(lock) {
    while (!done)
    lock.wait();
    }
    } catch (InterruptedException x) {
    Thread.currentThread().interrupt();
    }
    }


    (*)
    The doc mentions it explicitly:

    As in the one argument version, interrupts and *spurious wakeups* are
    possible, and this method should always be used in a *loop*:
    http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#wait()

    Bye

    markspace schrieb:
    > That's actually a fair point. "Doesn't throw" isn't the same as
    > "ignore." However, ReentrantLock is probably still better than the
    > following code.
    >
    >
    > /** A questionable method: wait without throwing InterruptedException. */
    > public void uninterruptableWait(Object c) {
    > boolean interrupted = false;
    > try {
    > synchronized(c) {
    > try {
    > c.wait();
    > return;
    > } catch(InterruptedExcpetion ex ) {
    > interrupted = true;
    > }
    > }
    > } finally {
    > if(interrupted == true ) Thread.currentThread().interrupt();
    > }
    > }
    >
     
    Jan Burse, Sep 25, 2012
    #10
  11. Jan Burse Guest

    Jan Burse schrieb:
    > If it doesn't we cannot distinguish between a spurious wait (*) and
    > a wait by a notify.


    Corr.:
    If it doesn't we cannot distinguish between a spurious wakeup (*) and
    a wakeup by a notify.

    This is how the notify should be done, it needs also to
    use a synchronized():

    public void signalDone() {
    synchronized(lock) {
    done=true;
    lock.notifyAll();
    }
    }

    I prefer using notifyAll() from the beginning, since the condition
    that is signaled can get more and more complex over the time, and
    it might be that some threads will or will not react to it, so that
    notify() might pick one which will not react, and then the signal
    is spurious.

    Bye
     
    Jan Burse, Sep 25, 2012
    #11
  12. Jan Burse Guest

    Jan Burse schrieb:
    > that is signaled can get more and more complex over the time

    Code evolution.
     
    Jan Burse, Sep 25, 2012
    #12
  13. markspace Guest

    On 9/25/2012 1:47 PM, Jan Burse wrote:
    > Hi,
    >
    > Anyway the code that will notify c should also set some state.
    > If it doesn't we cannot distinguish between a spurious wait (*) and
    > a wait by a notify. So code must use a loop, and to not confuse
    > the done flag and loop breaking, we can just shift the try/catch
    > outside of the loop, and even outside of the synchronized to
    > minimize the monitor region:
    >
    > public boolean done; /* should be set by the thread that
    > notifies the lock */



    While I agree this works in some cases, I don't think it works for the
    OP. He's trying to simulate a spin lock, by extending the Lock
    interface. "done" is signaled by calling unlock() on that interface.

    I don't see how that can be done reliably.

    public class SpinLock implements Lock {
    private volatile boolean done;
    private final Object lock = new Object();

    public void lock() {
    // ignore the "spin lock" for now
    try {
    synchronized( lock ) {
    while( !done ) lock.wait();
    }
    } catch( InterruptedException ex ) {
    Thread.currentThread().interrupt();
    }
    // how does "done" get reset?
    }

    public void unlock() {
    synchronized( lock ) {
    done = true;
    lock.notify();
    }
    }
    }

    I don't see how "done" can be reliably reset. There's going to be a
    race condition somewhere. Might as well use ReetrantLock, it solves
    this problem.

    (You can solve it too, but you're basically cutting and pasting code
    from ReetrantLock at that point. Easier to just reuse code.)
     
    markspace, Sep 25, 2012
    #13
  14. Jan Burse Guest

    markspace schrieb:
    >> public boolean done; /* should be set by the thread that
    >> notifies the lock */

    >
    >
    > While I agree this works in some cases,


    Just replace done by the <cond> you want to anyway check.
    The main point is that you cannot go, since factoring
    out the programming pattern works not:


    synchronized (c) {
    while (!<cond>)
    uninterruptableWait(c);
    }


    But rather simply apply the programming pattern:

    try {
    synchronized (c) {
    while (!<cond>)
    c.wait();
    }
    } catch( InterruptedException ex ) {
    Thread.currentThread().interrupt();
    }

    Or if you want to scare the hell out of your clients, use:

    public interface Predicate {
    public boolean _true(Object c);
    }

    public void uninterruptableWait(Object c, Predicate p) {
    try {
    synchronized (c) {
    while (!p._true(c))
    c.wait();
    }
    } catch( InterruptedException ex ) {
    Thread.currentThread().interrupt();
    }
    }

    Then what you call "SpinLock", but what I would call
    "OneTimeLock". Can be implemented as follows:

    public void waitDone() {
    uninterruptableWait(lock,new Predicate() {
    return done;
    });
    }

    Of course you can turn a "OneTimeLock" into a "ManyTimeLock",
    for example. You can reset the done inside the synchronized
    of the waitDone(). The synchronized will assure that when
    you leave the synchronized the done=false holds, since no
    other thread will interfer while inside the synchronized
    and after the wait():

    public void waitDone() {
    try {
    synchronized( lock ) {
    while( !done )
    lock.wait();
    done=false;
    }
    } catch( InterruptedException ex ) {
    Thread.currentThread().interrupt();
    }
    }

    To abstract this pattern we would need:


    public interface Predicate {
    public boolean _true(Object c);
    }

    public interface Action{
    public boolean perform(Object c);
    }

    public void uninterruptableWait(Object c, Predicate p, Action a) {
    try {
    synchronized (c) {
    while (!p._true(c))
    c.wait();
    a.perform(c);
    }
    } catch( InterruptedException ex ) {
    Thread.currentThread().interrupt();
    }
    }

    And one can then do:

    public void waitDone() {
    uninterruptableWait(lock,new Predicate() {
    return done;
    }, new Action() {
    done=false;
    });
    }

    Eagerly avaiting JDK 7 lambdas...

    Bye
     
    Jan Burse, Sep 25, 2012
    #14
  15. Jan Burse Guest

    Jan Burse schrieb:
    >
    > Eagerly avaiting JDK 7 lambdas...

    Oops, JDK 8, 9, or whatever...
     
    Jan Burse, Sep 25, 2012
    #15
  16. Jan Burse Guest

    Jan Burse schrieb:
    > public void waitDone() {
    > uninterruptableWait(lock,new Predicate() {
    > return done;
    > }, new Action() {
    > done=false;
    > });
    > }
    >
    > Eagerly avaiting JDK 7 lambdas...


    You see, I didn't write it right, it is of course:


    public void waitDone() {
    uninterruptableWait(lock,new Predicate() {
    public _true(Object c) {
    return done;
    }
    }, new Action() {
    public perform(Object c) {
    done=false;
    }
    });
    }

    So since there are not yet lambdas, we have to
    write the method names of the interfaces. But
    JDK 8 lambdas would exactly do this for us,
    fill in the blanks. Sadly I don't currently
    find the blog by John Rose that explains it.

    Bye
     
    Jan Burse, Sep 25, 2012
    #16
  17. Jan Burse Guest

    Jan Burse, Sep 25, 2012
    #17
  18. Jan Burse Guest

    Jan Burse schrieb:
    > Hi,
    >
    > markspace schrieb:
    >> I don't see how "done" can be reliably reset. There's going to be a
    >> race condition somewhere. Might as well use ReetrantLock, it solves
    >> this problem.

    >
    > Do you refer to:
    >
    > Condition.awaitUninterruptibly() ?
    > http://docs.oracle.com/javase/1.5.0...t/locks/Condition.html#awaitUninterruptibly()
    >
    >
    > Didn't look yet at the source. What is the design?
    >
    > Bye


    I guess if we were to allow to introduce a data structure
    Condition or so, we could also do the same. Provide an
    uninterruptableWait on this Condition. So we would again
    divert from the original:

    public static void uninterruptableWait(Object c);

    And either have:

    public static void uninterruptableWait(Condition c);

    Or:

    public class Condition;
    public void uninterruptableWait();

    Simplest datastructure would be:

    public class Condition {
    public Object lock = new Object();
    public boolean done;
    }

    Bye
     
    Jan Burse, Sep 25, 2012
    #18
  19. Jan Burse Guest

    But the programming pattern for awaitUninterruptibly() would
    be different from uninterruptableWait(). Since the spec says
    that spurious wakeups are still posible. So I guess something
    along:

    while (!(flag=Thread.interrupted()) && !<cond>)
    awaitUninterruptibly();
    if (!flag)
    <action>
    } else {
    Thread.currentThread().interrupt();
    }

    Ugly


    Jan Burse schrieb:
    > Hi,
    >
    > markspace schrieb:
    >> I don't see how "done" can be reliably reset. There's going to be a
    >> race condition somewhere. Might as well use ReetrantLock, it solves
    >> this problem.

    >
    > Do you refer to:
    >
    > Condition.awaitUninterruptibly() ?
    > http://docs.oracle.com/javase/1.5.0...t/locks/Condition.html#awaitUninterruptibly()
    >
    >
    > Didn't look yet at the source. What is the design?
    >
    > Bye
    >
    >
    >
    >
     
    Jan Burse, Sep 25, 2012
    #19
  20. markspace Guest

    On 9/25/2012 2:55 PM, Jan Burse wrote:

    > Ugly
    >



    Truly! And it only took you eight replies, mostly to your own posts, to
    arrive at that idea. I think I mentioned ReetrantLock up thread? And
    you should look at the source of that class, it's very interesting.
     
    markspace, Sep 25, 2012
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Sylwia
    Replies:
    2
    Views:
    423
    Nazgul
    Jan 8, 2004
  2. Nazgul
    Replies:
    0
    Views:
    808
    Nazgul
    Jan 8, 2004
  3. Hartin, Brian
    Replies:
    15
    Views:
    298
    Jan T.
    Feb 23, 2011
  4. Piotr Dobrogost
    Replies:
    0
    Views:
    89
    Piotr Dobrogost
    Feb 19, 2013
  5. Demian Brecht
    Replies:
    0
    Views:
    143
    Demian Brecht
    Feb 19, 2013
Loading...

Share This Page