InterruptedException should not be swallowed

[jhericks]

In AbstractSQSConnector.receiveSQSMessage, you do a Thread.sleep() in a while(true) loop. If you get an InterruptedException while sleeping, you do not re-interrupt the thread. This is bad practice. (See http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html for why.) The practical effect is that I have had this library deployed in a tomcat application and tomcat won't shut down gracefully because this thread will never get killed nicely.

Also, there is no reason for the warning log as getting interrupted is a perfectly normal occurrence. If there is a reason you want to see the stack trace, switch the log level to debug.

[carterpage]

Thanks for finding this. We'll get it fixed.

Sent from my iPhone

On Jan 21, 2013, at 2:14 AM, jhericks notifications@github.com wrote:

In AbstractSQSConnector.receiveSQSMessage, you do a Thread.sleep() in a while(true) loop. If you get an InterruptedException while sleeping, you do not re-interrupt the thread. This is bad practice. (See http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html for why.) The practical effect is that I have had this library deployed in a tomcat application and tomcat won't shut down gracefully because this thread will never get killed nicely.

Also, there is no reason for the warning log as getting interrupted is a perfectly normal occurrence. If there is a reason you want to see the stack trace, switch the log level to debug.

—
Reply to this email directly or view it on GitHub.

[jhericks]

Thanks.

[carterpage]

Pushing out 1.1.1 with fix. Will add comment when available in maven.

[carterpage]

1.1.1 is available on maven. @jhericks, please confirm if this fixes your issue.

[jhericks]

This should resolve the specific issue of Tomcat not stopping gracefully, but in the NevadoSession.stop() method, you still catch an InterruptedException and throw a new JMSException. When the InterruptedException is thrown, the "interrupted" flag on the thread is unset, so the thread no longer knows that it has been interrupted. You need to either re-throw the exception or reset the interrupt flag on the thread (Thread.currentThread().interrupt()).

[carterpage]

Thanks for pointing that one out too. It's very unlikely to be encountered, so we'll wait for the next release to push out a fix.

[jhericks]

Sure.