You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2020/09/08 19:15:49 UTC
[GitHub] [storm] agresch opened a new pull request #3332: STORM-3696 don't ignore InterruptedException
agresch opened a new pull request #3332:
URL: https://github.com/apache/storm/pull/3332
## What is the purpose of the change
It's not safe to assume InterruptedException is ignorable.
## How was the change tested
Rebuilt storm-client
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] kishorvpatil commented on pull request #3332: STORM-3696 don't ignore InterruptedException
Posted by GitBox <gi...@apache.org>.
kishorvpatil commented on pull request #3332:
URL: https://github.com/apache/storm/pull/3332#issuecomment-689106660
Good catch!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] bipinprasad commented on a change in pull request #3332: STORM-3696 don't ignore InterruptedException
Posted by GitBox <gi...@apache.org>.
bipinprasad commented on a change in pull request #3332:
URL: https://github.com/apache/storm/pull/3332#discussion_r485171932
##########
File path: storm-client/src/jvm/org/apache/storm/daemon/supervisor/ClientSupervisorUtils.java
##########
@@ -71,7 +71,9 @@ public static int processLauncherAndWait(Map<String, Object> conf, String user,
try {
process.waitFor();
} catch (InterruptedException e) {
- LOG.info("{} interrupted.", logPreFix);
+ LOG.warn("{} interrupted.", logPreFix);
+ Thread.currentThread().interrupt();
+ throw new IOException(logPreFix + " interrupted", e);
Review comment:
Need a process.destroy() here before throwing the IOException, so the cleanup can happen.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch commented on pull request #3332: STORM-3696 don't ignore InterruptedException
Posted by GitBox <gi...@apache.org>.
agresch commented on pull request #3332:
URL: https://github.com/apache/storm/pull/3332#issuecomment-689741510
squashed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [storm] agresch merged pull request #3332: STORM-3696 don't ignore InterruptedException
Posted by GitBox <gi...@apache.org>.
agresch merged pull request #3332:
URL: https://github.com/apache/storm/pull/3332
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org