Maciej W. Rozycki wrote:
--- lib/remote.exp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)dejagnu-remote-close-wait-kill-catch.diff Index: dejagnu/lib/remote.exp =================================================================== --- dejagnu.orig/lib/remote.exp +++ dejagnu/lib/remote.exp @@ -113,7 +113,10 @@ proc close_wait_program { program_id pid # We reaped the process, so cancel the pending force-kills, as # otherwise if the PID is reused for some other unrelated # process, we'd kill the wrong process. - exec sh -c "exec > /dev/null 2>&1 && kill -9 $exec_pid" + # + # Use `catch' in case the force-kills have completed, so as not + # to cause TCL to choke if `kill' returns a failure. + catch "exec sh -c \"exec > /dev/null 2>&1 && kill -9 $exec_pid\"" }return $res
That line is starting to look needlessly complicated. Does {catch {exec sh -c "kill -9 $exec_pid" >& /dev/null} also work? (I am using the outermost braces to denote quoting a piece of Tcl code inline in running text.) The "catch" command recursively invokes the interpreter in the same context, so there is no need for variables to be substituted before invoking catch(n). As a matter of modern Tcl (8.0+) style, the script argument to catch(n) should be in braces. If I understand correctly, a braced script argument to catch(n) will be byte-compiled when the surrounding procedure is loaded, while other scripts must be parsed at runtime. When fixing bugs that only bite under heavy load, efficiency should be a consideration. (I have spent many hours reading the Tcl reference manuals; hopefully others can also benefit from my experience here.)
Modern Tcl exec(n) also allows I/O redirection, so the only reason to invoke a shell at all is to support systems where kill(1) is only available as a shell builtin. This does not include GNU/Linux (where /bin/kill is provided by util-linux) and probably should not affect GNU at all. (If it does, either complain until "util-hurd" is released or get bash to invoke a builtin if run under the name thereof.) If we can rely on kill(1) being available as an executable on the PATH, it can be simplified further to {catch {exec kill -9 $exec_pid >& /dev/null}}. (Outermost braces again denote quoting Tcl code inline.)
Most of the complexity in this code is traceable back to the "Initial revision" and there are comments about using the shell instead of Expect to work around I/O bugs in "older CYGWIN" (added in commit 4306dace39d6628ca4105c672b1497c3c75ee745, but acknowledged as a "late" merge of a change dated 2001-11-12). On one hand, we should be able to move on from compatibility concerns with systems that ancient, but on the other hand, the unit tests for this module (in testsuite/runtest.libs/remote.test) are currently (still) a stub that does nothing, so caution is still the watchword.
-- Jacob
