[jira] [Comment Edited] (CASSANDRA-5623) Cleanup interruption handling

["Mikhail Mazursky (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-5623?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13681053#comment-13681053 ] 

Mikhail Mazursky edited comment on CASSANDRA-5623 at 6/12/13 8:52 AM:
----------------------------------------------------------------------

AFAIK there are two basic ways of dealing with interruption - ignore it (remember that you were interrupted, continue with the task and restore the interruption state after you're done) or propagate it as InterruptedException up the call chain (rarely it's wrapped in another exception when there is no other way e.g. InterruptedIOException). Both ways allow calling code to learn that the Thread was interrupted.

Wrapping IE in AssertionError/RuntimeException is clearly wrong - interruption is not an emergency way to stop thread, it's a polite way to ask to consider stopping current activity, which can be ignored if you can't or don't want to handle it.

But, yes, this patch changes semantics/behaviour. Probably in some places code can/should be refactored to propagate that exception instead of ignoring it.

The intention of this patch is to make code more correct, improve it's quality. And also I'm learning the codebase that way.
                
      was (Author: ash2k):
    AFAIK there are two basic ways of dealing with interruption - ignore it (remember that you were interrupted, continue with the task and restore the interruption state after you're done) or propagate it as InterruptedException up the call chain (rarely it's wrapped in another exception when there is no other way e.g. InterruptedIOException). Both ways allow calling code to learn that the Thread was interrupted.

Wrapping IE in AssertionError/RuntimeException is clearly wrong - interruption is not an emergency way to stop thread, it's a polite way to ask to consider stopping current activity, which can be ignored if you can't or don't want to handle it.

But, yes, this patch changes semantics/behaviour. Probably in some places code can be refactored to prapagate that exception instead of ignoring it.

The intention of this patch is to make code more correct, improve it's quality. And also I'm learning the codebase that way.
                  
> Cleanup interruption handling
> -----------------------------
>
>                 Key: CASSANDRA-5623
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-5623
>             Project: Cassandra
>          Issue Type: Improvement
>            Reporter: Mikhail Mazursky
>            Assignee: Mikhail Mazursky
>            Priority: Minor
>             Fix For: 2.0
>
>         Attachments: trunk-5623.txt
>
>
> There are a lot of catch-wrap-throw pattern occurances in code with InterruptedException. I cleaned up some of them but not all - in some places I don't know enough about code to decide if it's correct thing to do.
> Important: I also fixed possibility of wrong behaviour in case of spurious wakeup in AsyncOneResponse.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira