Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please handle InterruptedExceptions correctly #52

Closed
vfelberg opened this issue May 4, 2015 · 7 comments
Closed

Please handle InterruptedExceptions correctly #52

vfelberg opened this issue May 4, 2015 · 7 comments

Comments

@vfelberg
Copy link

vfelberg commented May 4, 2015

Class com.rabbitmq.utility.BlockingCell has several while loops catching InterruptedExceptions and ignoring them. This makes it impossible to shutdown the app correctly; you have to kill it brutally. The correct handling of an InterruptedException in a while loop would be:

try {
  ...
} catch (InterruptedException e) {
  Thread.currentThread().interrupt();
  return;
}

There are probably other classes swallowing InterruptedExceptions as well; I'm just reporting the one I found while analyzing what causes our app to hang.

For background info on thread interruption, see e.g.: http://www.ibm.com/developerworks/library/j-jtp05236/

@michaelklishin
Copy link
Member

@vfelberg yes, you are absolutely correct. By the way, you can submit a pull request that does the changes (against the stable branch), we will evaluate and merge it :)

@vfelberg
Copy link
Author

vfelberg commented May 4, 2015

Ok, I'll make the fix; it's an easy one :-)

@michaelklishin michaelklishin added this to the 3.5.2 milestone May 4, 2015
@michaelklishin
Copy link
Member

I'd like to get this into 3.5.2, so we'll wait for a PR (or a note that we should handle it in our own).

@vfelberg
Copy link
Author

vfelberg commented May 5, 2015

Unfortunately, it's not so easy to fix as I thought. I found in total 14 places catching InterruptedException; some of them trivial, most of them not. I guess, some of the core developers need to have a look at those places.

@michaelklishin
Copy link
Member

@vfelberg I will.

@michaelklishin
Copy link
Member

@vfelberg can you please reproduce the case when you have your app hanging on shutdown and post a thread dump here?

@michaelklishin
Copy link
Member

@vfelberg can you please build from source on the rabbitmq-java-client-52 branch and give this updated version a try? See bin/dist after ant dist finishes. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants