Keepalive thread's interrupt signal may be catched when it is sleeping

[gatesking]

In Keepalive.java:

try {
            while (!isInterrupted()) {
                final int hi = getPositiveInterval();
                if (conn.getTransport().isRunning()) {
                    log.debug("Sending keep-alive since {} seconds elapsed", hi);
                    doKeepAlive();
                }
                **Thread.sleep(hi * 1000);**
            }
        } catch (Exception e) {
            // If we weren't interrupted, kill the transport, then this exception was unexpected.
            // Else we're in shutdown-mode already, so don't forcibly kill the transport.
            if (!isInterrupted()) {
                conn.getTransport().die(e);
            }
        }

when keepalive interrupt is called in ConnectionImpl.notifyError, the interrupt signal may be catched by Thread.sleep, as a result, it will still go into conn.getTransport().die(e).
I suggest catch interrupt signal will be better, am I right?

try {
            while (!isInterrupted()) {
                final int hi = getPositiveInterval();
                if (conn.getTransport().isRunning()) {
                    log.debug("Sending keep-alive since {} seconds elapsed", hi);
                    doKeepAlive();
                }
                Thread.sleep(hi * 1000);
            }
        } **catch (InterruptedException e) {
                // do nothing
        }** catch (Exception e) {
            // If we weren't interrupted, kill the transport, then this exception was unexpected.
            // Else we're in shutdown-mode already, so don't forcibly kill the transport.
            if (!isInterrupted()) {
                conn.getTransport().die(e);
            }
        }

Thanks!

[hierynomus]

Yes, you're right. Please send a PR 2016-11-29 15:09 GMT+01:00 gatesking <notifications@github.com>:

…

In Keepalive.java: try { while (!isInterrupted()) { final int hi = getPositiveInterval(); if (conn.getTransport().isRunning()) { log.debug("Sending keep-alive since {} seconds elapsed", hi); doKeepAlive(); } **Thread.sleep(hi * 1000);** } } catch (Exception e) { // If we weren't interrupted, kill the transport, then this exception was unexpected. // Else we're in shutdown-mode already, so don't forcibly kill the transport. if (!isInterrupted()) { conn.getTransport().die(e); } } when keepalive interrupt is called in ConnectionImpl.notifyError, the interrupt signal may be catched by Thread.sleep, as a result, it will still go into conn.getTransport().die(e). I suggest this will be better, am I right? try { while (!isInterrupted()) { final int hi = getPositiveInterval(); if (conn.getTransport().isRunning()) { log.debug("Sending keep-alive since {} seconds elapsed", hi); doKeepAlive(); } Thread.sleep(hi * 1000); } } **catch (InterruptedException e) { // do nothing }** catch (Exception e) { // If we weren't interrupted, kill the transport, then this exception was unexpected. // Else we're in shutdown-mode already, so don't forcibly kill the transport. if (!isInterrupted()) { conn.getTransport().die(e); } } — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#283>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHLo4j8Yg2WPk6kLKq0NvfCcMzj8EsVks5rDDIlgaJpZM4K_Ayo> .

[hierynomus]

Merged #284, thanks!