[jboss-jira] [JBoss JIRA] Commented: (JBMESSAGING-1637) Interruption is busted

Tue Jun 2 10:06:57 EDT 2009

    [ https://jira.jboss.org/jira/browse/JBMESSAGING-1637?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12470237#action_12470237 ] 

David Lloyd commented on JBMESSAGING-1637:
------------------------------------------

Tim, being able to handle interruption is not optional, something that you support or don't support.  When you run in a container it's just a matter of time before something trips the interrupt status - your caller might use it, or a callee might use it.  Not supporting interruption is a bug.  My patch simply adopts a policy of deferring the interrupt in a safe manner, which fixes the bug.  Now if you want interruption to prematurely cancel an event in a clean way, that is a feature that you are free to implement if you wish.

Howard, in response to 1., *everyone* that invokes an interruptible operation has the responsibility of resetting the interrupted status.  If there is no sane way to handle interruption (because, for example, someone thought it might be a good idea to throw Exception from a method), then the only other option is to ignore it but *preserve* the flag setting, so the caller has a chance to handle it.  Yes, this means that things may keep running after interruption.  But this is what your options are unless you're willing to go through and strip out all those "throws Exceptions", replace them with "throws InterruptedException, XxxxException, ..." *and* locate all the call sites and remove *their* "catch (Exception e)" and replace them with interrupt-aware handlers, and *then* go through and apply sane interrupt handling to all those call sites.

The goal here isn't to make JBM interruptible - that would be a lot more work - but to fix the bug that it has no strategy whatsoever to handle interruption, resulting in all kinds of possible problems.  If JBM was using j.u.c instead of oswego, I would have simply replaced the lock calls with uninterruptible lock calls.

As for (2), you are incorrect.  InterruptedException indicates that the thread was interrupted while acquiring the lock.  As I've said many times before, when you invoke an operation that can throw InterruptedException, your choices are to preserve the interrupt status and retry the operation, preserve interrupt status and throw some other exception, or rethrow InterruptedException itself *after* correctly handing the interrupt situation by aborting what you're doing in a clean manner (whether that be rolling back your operation or whatever).  Since JBM 1.x was obviously not designed with interruption in mind, the first option is the safest.  You may disagree and decide to go ahead and refactor all these methods and their call sites.  But remember this.  Any operation which throws InterruptedException should be *retryable* (in other words, you must give the caller the opportunity to decide whether they care that an operation was interrupted - maybe they don't and they want to retry it anyway).  If you make your operation interruptible and it is not retryable, you should not be throwing InterruptedException, but instead, re-set the interrupt status of the current thread and throw some other exception.

> Interruption is busted
> ----------------------
>
>                 Key: JBMESSAGING-1637
>                 URL: https://jira.jboss.org/jira/browse/JBMESSAGING-1637
>             Project: JBoss Messaging
>          Issue Type: Bug
>          Components: Messaging Core
>    Affects Versions: 1.4.1.GA, 1.4.3.GA
>            Reporter: David Lloyd
>            Assignee: Howard Gao
>             Fix For: 1.4.5.GA
>
>         Attachments: fix-interruption.patch
>
>
> Interruption handling is way wrong in MessagingPostOffice.  Attaching a patch that at least prevents large amounts of exception spewage when a JBM thread is interrupted.  Please check it over and apply.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira