You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Patrick Moore (JIRA)" <ji...@apache.org> on 2008/07/10 23:24:31 UTC

[jira] Created: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

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


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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612814#action_12612814 ] 

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

OK. Now I understand the problem a little better. I think IO reactor should never pass CancelledKeyException to the exception handler. CancelledKeyExceptions must always be handled internally. This will also eliminate the need for #handleTerminatingThrowable method. Could you live with this approach?

> Also the IOReactor just quietly shutsdown with out saying why,.. 

This certainly should not be happening. If I/O reactor terminates abnormally it is definitely expected to throw either an IOReactorException or a RuntimeException. Could you please give some more details as to when such condition can occur?

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
>
> 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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613258#action_12613258 ] 

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

Patrick,

IOReactorEH defines a very simple contract: it can declare certain classes of exceptions as handled, which would otherwise be considered fatal. That is it. If a caller does not act on this information this is not a problem of the IOReactorEH interface. 

Anyway, you are very welcome to demonstrate a better way of doing it by submitting a patch.

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
>             Fix For: 4.0-beta3
>
>         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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612666#action_12612666 ] 

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

Patrick,

What changes do suggest we should make? Could you please attach stack traces of incorrectly handled CancelledKeyExceptions?

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
>
> 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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Patrick Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612976#action_12612976 ] 

Patrick Moore commented on HTTPCORE-165:
----------------------------------------

Hi Oleg --

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. Also the question of InterruptedIOException also exists.

Still am thinking that that the IOReactorExceptionHandler will want to deal with the question of exception propgation as a separate decision from terminating the IOReactor. Making the 2 equivalent works for the current implementation, but may not work for a custom IOReactor implementation or the future version of BaseIOReactor. 

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?

This would happen if we provide our own custom IOReactor. The custom reactor may handle certain exceptions in a way that does not result in termination. 

For example, the custom reactor gets an IOException because the remote server dropped the connection. The custom IOReactor notifies the IOReactorExceptionHandler. The IOReactorExceptionHandler logs the issue and indicates "rethrow". the custom IOReactor does its recovery and retries. Another error occurs. This time the IOReactorEH wraps exception and throws?, (returns?) the new exception to be thrown. This new exception is not caught until the outer loop. At this point,  the custom IOReactor calls:

 IOReactorStatus IOReactorEH.handleTerminatingThrowable(Throwable throwable) ;

The IOReactorEH would return the new IOReactorStatus. If the IOReactorStatus returned is RESTART then the IOReactor would reinitialize and resume running. So conceptually the IOReactor has "rebooted" itself.

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.

Continuing....

* the IOReactorEH also needs to know what is the operation being attempted that caused the exception. For example, if the IOReactor is doing a shutdown then the IOReactorExceptionHandler may chose to ignore all exceptions. This should also include things like the connection url, and other meaningful data. 
* the IOReactorEH should be passed the instance of the IOReactor to better support singleton IOReactorEH.

Summary:
1. existing "handle" methods should really return the exception to be thrown to allow for Exception wrapping.
  a. IOReactor should notify what operation was being attempted that caused the problem so IOReactorEH could do more intelligent logging and handling.
  b. IOReactor should pass 'this' to the IOReactorEH so the IOReactorEH can be a stateless singleton.
2. separating the concept of propagate exception from terminating IOReactor.
3. enable IOReactorEH to trigger a restart of the IOReactor.

----------------------

Pat

> 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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
    [ 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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Resolved: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski resolved HTTPCORE-165.
----------------------------------------

       Resolution: Fixed
    Fix Version/s: 4.0-beta3

Patrick

I committed the fix for CancelledKeyException problem. Many thanks for reporting it. I am going to close this issue. Please open separate tickets for other bugs you are aware of. Let's discuss possible API changes on the dev list.

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
>             Fix For: 4.0-beta3
>
>         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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Updated: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Oleg Kalnichevski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Oleg Kalnichevski updated HTTPCORE-165:
---------------------------------------

    Attachment: cancelledkey.patch

Patrick, 

How about the following patch? Could you please test it with your app?

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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Patrick Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12613231#action_12613231 ] 

Patrick Moore commented on HTTPCORE-165:
----------------------------------------

Not too inclined to get on yet another email list..

Final comment --- something to think about:

the IOReactorExceptionHandler is violating a good principal of interface design. IOReactorExceptionHandler is imposing an expected behavior on the CALLER -- not the IOReactorExceptionHandler implementation. If a class calls the IOReactorExceptionHandler the CALLER is expected to terminate. What!? Maybe the code that currently calls the IOReactorEH does ... but what happens if I implement a IOReactor that doesn't? Are bad things going to happen if my custom implementation of IOReactor calls your custom implementation of IOReactorExceptionHandler and then does NOT terminate?

Seems like you are imposing (unnecessary) conditions on the caller.





> 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
>             Fix For: 4.0-beta3
>
>         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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org


[jira] Commented: (HTTPCORE-165) Poor handling of CancelledKeyException with custom IOReactorExceptionHandler

Posted by "Patrick Moore (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/HTTPCORE-165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12612676#action_12612676 ] 

Patrick Moore commented on HTTPCORE-165:
----------------------------------------

Changing BaseIOReactor:

    protected void validate(final Set<SelectionKey> keys) {
        long currentTime = System.currentTimeMillis();
        if( (currentTime - this.lastTimeoutCheck) >= this.timeoutCheckInterval) {
            this.lastTimeoutCheck = currentTime;
            if (keys != null) {
                for (Iterator<SelectionKey> it = keys.iterator(); it.hasNext();) {
                    SelectionKey key = it.next();
                    try {
                        timeoutCheck(key, currentTime);
                    } catch(CancelledKeyException cancelledKeyException) {
                        it.remove();
                    }
                }
            }
        }

    protected void sessionClosed(final IOSession session) {
        try {
            this.eventDispatch.disconnected(session);
        } catch (RuntimeException ex) {
            try {
                handleRuntimeException(ex);
            } catch (CancelledKeyException cancelledKeyException) {
                // nothing to do here.
            }
        }
    }

--------------------------------------------------------------------------
IOReactorExceptionHandler:

    /**
     * This method is expected to examine the runtime exception passed as a parameter
     * and decide whether it is safe to continue execution of the I/O reactor
     *
     * @param ex potentially recoverable runtime exception
     * @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 {@link RuntimeException} <strike>and terminate</strike>

     * For CancelledKeyException, true should be returned. All other exceptions may terminate the IOReactor.
     */
    boolean handle(RuntimeException ex);

------------------------------------------------------------------
I would suggest adding:

   boolean handleTerminatingThrowable(Throwable throwable)

This way you can cleanly separate a decision about terminating the IOReactor from if the exception should be rethrown. This method would be called if the IOReactor thinks it should shutdown

Also the IOReactor just quietly shutsdown with out saying why,.. 

> 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
>
> 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 {@link 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: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org