[ 
https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613071#action_12613071
 ] 

Oleg Kalnichevski commented on HTTPCORE-165:
--------------------------------------------

Patrick


> Yeah. It looks like the patch would handle the big issues w.r.t. 
> CanceledKeyException. I agree 
> that the IOReactorExceptionHandler probably should not see this kind of 
> exception. 

All right. I'll go ahead and apply the patch

> Also the question of InterruptedIOException also exists. 

I think it makes sense to propagate InterruptedIOException to the caller and 
terminate the I/O reactor. I believe this is currently the case

> Right now -- the IOReactorExceptionHandler returns true if it wants the 
> exception rethrown with 
> the expectation that this will result in the IOReactor terminating. But how 
> do we handle the case 
> where the exception should be rethrown WITHOUT the expectation that the 
> IOReactor would terminate? 

I think you are missing an important point. I/O and protocol exceptions that 
occur on individual connections are are propagated to the I/O dispatcher, which 
in its turn propagate them to the protocol handler. Then it is up to the 
protocol handler to take corrective measures. Such I/O and protocol exceptions 
_cannot_ terminate the I/O reactor unless rethrown as runtime exceptions. On 
the contrary, exceptions passed to the IOReactorExceptionHandler affect the 
reactor as a whole and are deemed pretty much non recoverable and thus are 
meant to terminate the reactor. For instance, such global exceptions are a 
failure to open a new channel due to the system running out of file 
descriptors, a failure to allocate a particular port or an internal selector 
failure. 

We are providing IOReactorExceptionHandler mechanism mainly for two reasons: 
(1) some non-recoverable I/O exceptions can be considered non fatal in some 
contexts. For instance failure to open one of three ports, say 8080, 8081, and 
8082, should not result in shutting down of the entire I/O reactor; (2) there 
is no way of telling if a given custom runtime exception is a fatal one or not. 
This should be left up to the user to decide. 

So, one again the important bit is that the IOReactorExceptionHandler is NOT 
intended to handle I/O errors of individual connections. 

> This is critical in a server environment where modules will fail and need to 
> be restarted without requiring that the 
> entire application be restarted. Enabling this mini-reboot functionality is 
> extremely useful in maintaining uptime. 

In my opinion I/O reactors should never terminate if they are in a consistent 
state and can continue serving existing connections. I am not sure I see a 
difference between a mini-reboot and a restart of the reactor.

> 1. existing "handle" methods should really return the exception to be thrown 
> to allow for Exception wrapping. 

That would be useful, but I am not sure this warrants API breakage.

>  a. IOReactor should notify what operation was being attempted that caused 
> the problem so IOReactorEH could do 
> more intelligent logging and handling. 

If you want access to the execution context you should simply handle those 
exceptions on the protocol level. I/O reactors are meant to be protocol 
agnostic. 

> 2. separating the concept of propagate exception from terminating IOReactor.

See above. You really should be implementing error recovery mechanisms at the 
protocol level, leaving  IOReactorExceptionHandler as an absolutely last 
resort. 

> 3. enable IOReactorEH to trigger a restart of the IOReactor. 

I looked into this issue a while ago. I felt it would much cleaner to have I/O 
reactors restarted externally and an I/O reactor trying to re-initialize 
itself. In my opinion such control logic not should part of the reactor itself.

I suggest we should take this discussion to the dev list. JIRA is not the right 
place.

Oleg

> Poor handling of CancelledKeyException with custom IOReactorExceptionHandler 
> -----------------------------------------------------------------------------
>
>                 Key: HTTPCORE-165
>                 URL: https://issues.apache.org/jira/browse/HTTPCORE-165
>             Project: HttpComponents HttpCore
>          Issue Type: Bug
>          Components: HttpCore NIO
>    Affects Versions: 4.0-beta2
>            Reporter: Patrick Moore
>         Attachments: cancelledkey.patch
>
>
> IOReactorExceptionHandler documentation for handle(RuntimeException ex) is 
> misleading. Specifically the documentation says :
>      @return <code>true</code> if it is safe to ignore the exception 
>      and continue execution of the I/O reactor; <code>false</code> if the
>      I/O reactor must throw [EMAIL PROTECTED] RuntimeException} and terminate 
> However, a CancelledKeyException should be rethrown because the BaseIOReactor 
> and the AbstractIOReactor expect and handle CancelledKeyException correctly.
> BUT -- NOT always.
> Looking at the call stack for where 
> IOReactorExceptionHandler.handle(RuntimeException) is called reveals some 
> problems.
> If a custom IOReactorExceptionHandler was to return false (indicating that 
> the CanceledKeyException was to be rethrown or to do the rethrow itself) 
> WOULD result in the IOReactor being shutdown:
>   BaseIOReactor.sessionClosed(IOSession)
>   BaseIOReactor.timeoutCheck(SelectionKey, long)

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to