[jira] Created: (EXEC-46) Process.waitFor should clear interrupt status when throwing InterruptedException

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (EXEC-46) Process.waitFor should clear interrupt status when throwing InterruptedException

JIRA jira@apache.org
Process.waitFor should clear interrupt status when throwing InterruptedException
--------------------------------------------------------------------------------

                 Key: EXEC-46
                 URL: https://issues.apache.org/jira/browse/EXEC-46
             Project: Commons Exec
          Issue Type: Improvement
         Environment: any
            Reporter: nir
            Priority: Minor


Taken from - http://kylecartmell.com/?p=9

By convention, methods that throw InterruptedException reset the thread interrupt flag.
Unfortunately Process.waitFor didn't get that memo. (See Sun bug 6420270 - http://bugs.sun.com/view_bug.do?bug_id=6420270).
This is especially entertaining when a thread invokes multiple processes consecutively and calls waitFor for each of them;
After one call to waitFor is interrupted, future calls to waitFor from the same thread will immediately throw InterruptedException until the interrupt flag is cleared.

Process.waitFor should always be called from a try block whether InterruptedException is caught or not, with a corresponding finally block that calls Thread.interrupted to clear the interrupt flag.

So maybe the code in DefaultExecutor.executeInternal() method should change:
int exitValue = Executor.INVALID_EXITVALUE;
try {
  exitValue = process.waitFor();
} catch (InterruptedException e) {
  process.destroy();
}

Maybe there is a need to add
Thread.interrupted()
to the catch/finally block

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (EXEC-46) Process.waitFor should clear interrupt status when throwing InterruptedException

JIRA jira@apache.org

    [ https://issues.apache.org/jira/browse/EXEC-46?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12871255#action_12871255 ]

Siegfried Goeschl commented on EXEC-46:
---------------------------------------

Thanks for hint - I need to write a test case to see if commons-exec is affected

> Process.waitFor should clear interrupt status when throwing InterruptedException
> --------------------------------------------------------------------------------
>
>                 Key: EXEC-46
>                 URL: https://issues.apache.org/jira/browse/EXEC-46
>             Project: Commons Exec
>          Issue Type: Improvement
>         Environment: any
>            Reporter: nir
>            Assignee: Siegfried Goeschl
>            Priority: Minor
>
> Taken from - http://kylecartmell.com/?p=9
> By convention, methods that throw InterruptedException reset the thread interrupt flag.
> Unfortunately Process.waitFor didn't get that memo. (See Sun bug 6420270 - http://bugs.sun.com/view_bug.do?bug_id=6420270).
> This is especially entertaining when a thread invokes multiple processes consecutively and calls waitFor for each of them;
> After one call to waitFor is interrupted, future calls to waitFor from the same thread will immediately throw InterruptedException until the interrupt flag is cleared.
> Process.waitFor should always be called from a try block whether InterruptedException is caught or not, with a corresponding finally block that calls Thread.interrupted to clear the interrupt flag.
> So maybe the code in DefaultExecutor.executeInternal() method should change:
> int exitValue = Executor.INVALID_EXITVALUE;
> try {
>   exitValue = process.waitFor();
> } catch (InterruptedException e) {
>   process.destroy();
> }
> Maybe there is a need to add
> Thread.interrupted()
> to the catch/finally block

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (EXEC-46) Process.waitFor should clear interrupt status when throwing InterruptedException

JIRA jira@apache.org
In reply to this post by JIRA jira@apache.org

     [ https://issues.apache.org/jira/browse/EXEC-46?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siegfried Goeschl resolved EXEC-46.
-----------------------------------

    Fix Version/s: 1.1
       Resolution: Fixed

Wrote a test but at least under Mac OS X the problem did not appear with commons-exec. Having said that I'm resetting the Interrupted flag to be on the save side.

> Process.waitFor should clear interrupt status when throwing InterruptedException
> --------------------------------------------------------------------------------
>
>                 Key: EXEC-46
>                 URL: https://issues.apache.org/jira/browse/EXEC-46
>             Project: Commons Exec
>          Issue Type: Improvement
>         Environment: any
>            Reporter: nir
>            Assignee: Siegfried Goeschl
>            Priority: Minor
>             Fix For: 1.1
>
>
> Taken from - http://kylecartmell.com/?p=9
> By convention, methods that throw InterruptedException reset the thread interrupt flag.
> Unfortunately Process.waitFor didn't get that memo. (See Sun bug 6420270 - http://bugs.sun.com/view_bug.do?bug_id=6420270).
> This is especially entertaining when a thread invokes multiple processes consecutively and calls waitFor for each of them;
> After one call to waitFor is interrupted, future calls to waitFor from the same thread will immediately throw InterruptedException until the interrupt flag is cleared.
> Process.waitFor should always be called from a try block whether InterruptedException is caught or not, with a corresponding finally block that calls Thread.interrupted to clear the interrupt flag.
> So maybe the code in DefaultExecutor.executeInternal() method should change:
> int exitValue = Executor.INVALID_EXITVALUE;
> try {
>   exitValue = process.waitFor();
> } catch (InterruptedException e) {
>   process.destroy();
> }
> Maybe there is a need to add
> Thread.interrupted()
> to the catch/finally block

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.