You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Marco Ferrante (JIRA)" <ji...@apache.org> on 2009/01/05 17:21:44 UTC
[jira] Created: (EXEC-34) Race condition prevent watchdog working
using ExecuteStreamHandler
Race condition prevent watchdog working using ExecuteStreamHandler
------------------------------------------------------------------
Key: EXEC-34
URL: https://issues.apache.org/jira/browse/EXEC-34
Project: Commons Exec
Issue Type: Bug
Environment: Windows Vista 64bit, dual core CPU
Reporter: Marco Ferrante
Priority: Critical
Consider this test case (in _DefaultExecutorTest_ class):
{noformat}
/**
* Start a async process using a stream handler and terminate it manually
* before the watchdog timeout occurs
*/
public void testExecuteAsyncWithStreamHandlerAndUserTermination() throws Exception {
CommandLine cl = new CommandLine(foreverTestScript);
ExecuteWatchdog watchdog = new ExecuteWatchdog(Integer.MAX_VALUE);
PumpStreamHandler streamHanlder = new PumpStreamHandler(System.out, System.err);
exec.setStreamHandler(streamHanlder);
MockExecuteResultHandler handler = new MockExecuteResultHandler();
exec.execute(cl, handler);
// DON'T wait for script to run
//Thread.sleep(2000);
// teminate it
watchdog.destroyProcess();
assertTrue("Watchdog should have killed the process",watchdog.killedProcess());
}
{noformat}
It fails (at least in my environment) because when _watchdog.destroyProcess()_ is invoked the external process is not bound to the watchdog yet.
Although there are possible several workarounds, but all of them seem to me very intrusive in the code. So, I prefer some discussion before preparing and submitting a patch.
IMHO, the watchdog should handle a reference to the thread running the process, not to the process itself. In this way, interrupting signals can be transport using default _interrupt()_ method of class _Thread_.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (EXEC-34) Race condition prevent watchdog working
using ExecuteStreamHandler
Posted by "Konrad Windszus (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/EXEC-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12769177#action_12769177 ]
Konrad Windszus commented on EXEC-34:
-------------------------------------
The same applies to e.g. isWatching(). I first tried to wait for an asynchronous process to finish, with the help of ExecuteWatchdog.isWatching(), apparently with no success, due to this bug. You should at least document this flaw.
> Race condition prevent watchdog working using ExecuteStreamHandler
> ------------------------------------------------------------------
>
> Key: EXEC-34
> URL: https://issues.apache.org/jira/browse/EXEC-34
> Project: Commons Exec
> Issue Type: Bug
> Environment: Windows Vista 64bit, dual core CPU
> Reporter: Marco Ferrante
> Assignee: Siegfried Goeschl
> Priority: Critical
>
> Consider this test case (in _DefaultExecutorTest_ class):
> {noformat}
> /**
> * Start a async process using a stream handler and terminate it manually
> * before the watchdog timeout occurs
> */
> public void testExecuteAsyncWithStreamHandlerAndUserTermination() throws Exception {
> CommandLine cl = new CommandLine(foreverTestScript);
> ExecuteWatchdog watchdog = new ExecuteWatchdog(Integer.MAX_VALUE);
> PumpStreamHandler streamHanlder = new PumpStreamHandler(System.out, System.err);
> exec.setStreamHandler(streamHanlder);
> MockExecuteResultHandler handler = new MockExecuteResultHandler();
> exec.execute(cl, handler);
> // DON'T wait for script to run
> //Thread.sleep(2000);
> // teminate it
> watchdog.destroyProcess();
> assertTrue("Watchdog should have killed the process",watchdog.killedProcess());
> }
> {noformat}
> It fails (at least in my environment) because when _watchdog.destroyProcess()_ is invoked the external process is not bound to the watchdog yet.
> Although there are possible several workarounds, but all of them seem to me very intrusive in the code. So, I prefer some discussion before preparing and submitting a patch.
> IMHO, the watchdog should handle a reference to the thread running the process, not to the process itself. In this way, interrupting signals can be transport using default _interrupt()_ method of class _Thread_.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (EXEC-34) Race condition prevent watchdog working
using ExecuteStreamHandler
Posted by "Siegfried Goeschl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/EXEC-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12662929#action_12662929 ]
Siegfried Goeschl commented on EXEC-34:
---------------------------------------
To be honest I was aware of the issue therefore the "Thread.sleep" statements in the regression tests. Having said that I think this is an artificial use case since I have to wait for some time for any external process to start and finish before I consider the process "hanging".
> Race condition prevent watchdog working using ExecuteStreamHandler
> ------------------------------------------------------------------
>
> Key: EXEC-34
> URL: https://issues.apache.org/jira/browse/EXEC-34
> Project: Commons Exec
> Issue Type: Bug
> Environment: Windows Vista 64bit, dual core CPU
> Reporter: Marco Ferrante
> Assignee: Siegfried Goeschl
> Priority: Critical
>
> Consider this test case (in _DefaultExecutorTest_ class):
> {noformat}
> /**
> * Start a async process using a stream handler and terminate it manually
> * before the watchdog timeout occurs
> */
> public void testExecuteAsyncWithStreamHandlerAndUserTermination() throws Exception {
> CommandLine cl = new CommandLine(foreverTestScript);
> ExecuteWatchdog watchdog = new ExecuteWatchdog(Integer.MAX_VALUE);
> PumpStreamHandler streamHanlder = new PumpStreamHandler(System.out, System.err);
> exec.setStreamHandler(streamHanlder);
> MockExecuteResultHandler handler = new MockExecuteResultHandler();
> exec.execute(cl, handler);
> // DON'T wait for script to run
> //Thread.sleep(2000);
> // teminate it
> watchdog.destroyProcess();
> assertTrue("Watchdog should have killed the process",watchdog.killedProcess());
> }
> {noformat}
> It fails (at least in my environment) because when _watchdog.destroyProcess()_ is invoked the external process is not bound to the watchdog yet.
> Although there are possible several workarounds, but all of them seem to me very intrusive in the code. So, I prefer some discussion before preparing and submitting a patch.
> IMHO, the watchdog should handle a reference to the thread running the process, not to the process itself. In this way, interrupting signals can be transport using default _interrupt()_ method of class _Thread_.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (EXEC-34) Race condition prevent watchdog working
using ExecuteStreamHandler
Posted by "Siegfried Goeschl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/EXEC-34?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Siegfried Goeschl updated EXEC-34:
----------------------------------
Priority: Minor (was: Critical)
Changing priority to minor since this is an artificial problem not happening in production - nobody would start a subprocess and immediately kill it.
> Race condition prevent watchdog working using ExecuteStreamHandler
> ------------------------------------------------------------------
>
> Key: EXEC-34
> URL: https://issues.apache.org/jira/browse/EXEC-34
> Project: Commons Exec
> Issue Type: Bug
> Environment: Windows Vista 64bit, dual core CPU
> Reporter: Marco Ferrante
> Assignee: Siegfried Goeschl
> Priority: Minor
>
> Consider this test case (in _DefaultExecutorTest_ class):
> {noformat}
> /**
> * Start a async process using a stream handler and terminate it manually
> * before the watchdog timeout occurs
> */
> public void testExecuteAsyncWithStreamHandlerAndUserTermination() throws Exception {
> CommandLine cl = new CommandLine(foreverTestScript);
> ExecuteWatchdog watchdog = new ExecuteWatchdog(Integer.MAX_VALUE);
> PumpStreamHandler streamHanlder = new PumpStreamHandler(System.out, System.err);
> exec.setStreamHandler(streamHanlder);
> MockExecuteResultHandler handler = new MockExecuteResultHandler();
> exec.execute(cl, handler);
> // DON'T wait for script to run
> //Thread.sleep(2000);
> // teminate it
> watchdog.destroyProcess();
> assertTrue("Watchdog should have killed the process",watchdog.killedProcess());
> }
> {noformat}
> It fails (at least in my environment) because when _watchdog.destroyProcess()_ is invoked the external process is not bound to the watchdog yet.
> Although there are possible several workarounds, but all of them seem to me very intrusive in the code. So, I prefer some discussion before preparing and submitting a patch.
> IMHO, the watchdog should handle a reference to the thread running the process, not to the process itself. In this way, interrupting signals can be transport using default _interrupt()_ method of class _Thread_.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (EXEC-34) Race condition prevent watchdog working
using ExecuteStreamHandler
Posted by "Siegfried Goeschl (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/EXEC-34?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898631#action_12898631 ]
Siegfried Goeschl commented on EXEC-34:
---------------------------------------
Well, actually the sample is buggy since the following line is missing
exec.setWatchdog(watchdog)
which injects the newly created 'Process' instance into the watchdog and without *Process' instance 'watchdog.destroy()' has no effect
> Race condition prevent watchdog working using ExecuteStreamHandler
> ------------------------------------------------------------------
>
> Key: EXEC-34
> URL: https://issues.apache.org/jira/browse/EXEC-34
> Project: Commons Exec
> Issue Type: Bug
> Environment: Windows Vista 64bit, dual core CPU
> Reporter: Marco Ferrante
> Assignee: Siegfried Goeschl
> Priority: Minor
>
> Consider this test case (in _DefaultExecutorTest_ class):
> {noformat}
> /**
> * Start a async process using a stream handler and terminate it manually
> * before the watchdog timeout occurs
> */
> public void testExecuteAsyncWithStreamHandlerAndUserTermination() throws Exception {
> CommandLine cl = new CommandLine(foreverTestScript);
> ExecuteWatchdog watchdog = new ExecuteWatchdog(Integer.MAX_VALUE);
> PumpStreamHandler streamHanlder = new PumpStreamHandler(System.out, System.err);
> exec.setStreamHandler(streamHanlder);
> MockExecuteResultHandler handler = new MockExecuteResultHandler();
> exec.execute(cl, handler);
> // DON'T wait for script to run
> //Thread.sleep(2000);
> // teminate it
> watchdog.destroyProcess();
> assertTrue("Watchdog should have killed the process",watchdog.killedProcess());
> }
> {noformat}
> It fails (at least in my environment) because when _watchdog.destroyProcess()_ is invoked the external process is not bound to the watchdog yet.
> Although there are possible several workarounds, but all of them seem to me very intrusive in the code. So, I prefer some discussion before preparing and submitting a patch.
> IMHO, the watchdog should handle a reference to the thread running the process, not to the process itself. In this way, interrupting signals can be transport using default _interrupt()_ method of class _Thread_.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.