> -----Original Message-----
> From: Martin Sebor [mailto:[EMAIL PROTECTED]
> Sent: Friday, August 18, 2006 4:01 AM
> To: [email protected]
> Subject: Re: [patch] exec utility windows port
>
> Andrew Black wrote:
> > And, take 2 on the patch. I believe I've resolved the
> issue with soft
> > killing the child processes, but I am not completely happy with it.
> > Part of this reason for this unhappiness is that the MSDN
> > documentation on process creation flags seems to indicate that the
> > Control-C signal handling is disabled on the new process group.
> > Unfortunately, I don't think there's any way around this. Still, I
> > don't see error messages when a child process is killed, so
> I think it will work.
>
> Looks good! Committed thus:
> http://svn.apache.org/viewvc?rev=432452&view=rev
>
> Let's see what Farid says -- I assume this will be integrated
> into the next version of the Windows build infrastructure, right?
I have looked to the path today and found some problems
(the patch, fixes that problems is attached).
ChangeLog for patch:
* exec.cpp [_WIN32 || _WIN64] (warn_last_error):
(exec_file): Added wait after sending Ctrl+Break signal;
added wait after TerminateProcess(); Closed handles of
child process.
exec.cpp, function warn_last_error(): if FORMAT_MESSAGE_ALLOCATE_BUFFER
is used then lpBuffer points to the place where pointer to the allocated
buffer is stored.
----------------------------
LPTSTR error_text = 0;
if (FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER |
FORMAT_MESSAGE_FROM_SYSTEM, NULL, error,
MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
(LPTSTR)&error_text, 0, NULL)) {
warn ("%s failed with error %d: %s\n", action, error,
error_text);
LocalFree (error_text);
}
else {
warn ("%s failed with error %d. Additionally, FormatMessage "
"failed with error %d.\n", action, error,
GetLastError ());
}
-------------------------------
exec.cpp, function exec_file(): missed WaitForSingleObject() call after
sending CTRL_BREAK_EVENT signal. At the line GenerateConsoleCtrlEvent
(CTRL_BREAK_EVENT,...) wait_code != WAIT_TIMEOUT and TerminateProcess()
will be called immidiately; the handles child.hProcess and child.hThread
should be closed.
-------------------------------
if (VER_PLATFORM_WIN32_NT == OSVer.dwPlatformId) {
if (0 == GenerateConsoleCtrlEvent (CTRL_C_EVENT,
child.dwProcessId))
warn_last_error ("Sending child process Control-C");
wait_code = WaitForSingleObject (child.hProcess, 1000);
if (WAIT_TIMEOUT != wait_code) {
if (WAIT_OBJECT_0 == wait_code) {
if (0 == GetExitCodeProcess (child.hProcess,
&status.status)) {
warn_last_error ("Retrieving child process exit code");
status.status = -1;
}
status.error = 1;
}
else {
status.status = -1;
status.error = warn_last_error ("Waiting for child
process");
}
return status;
}
if (0 == GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT,
child.dwProcessId))
warn_last_error ("Sending child process Control-Break");
if (WAIT_TIMEOUT != wait_code) {
if (WAIT_OBJECT_0 == wait_code) {
if (0 == GetExitCodeProcess (child.hProcess,
&status.status)) {
warn_last_error ("Retrieving child process exit code");
status.status = -1;
}
status.error = 2;
}
else {
status.status = -1;
status.error = warn_last_error ("Waiting for child
process");
}
return status;
}
}
/* Then hard kill the child process */
if (0 == TerminateProcess (child.hProcess, 3))
warn_last_error ("Terminating child process");
if (0 == GetExitCodeProcess (child.hProcess, &status.status)) {
warn_last_error ("Retrieving child process exit code");
status.status = -1;
}
And I suggest to add WaitForSingleObject() after a TerminateProcess()
because the process may not be terminated yet at the moment
TerminateProcess() returns (MSDN says: "TerminateProcess initiates
termination and returns immediately.").
Farid.
Index: exec.cpp
===================================================================
--- exec.cpp (revision 432584)
+++ exec.cpp (working copy)
@@ -843,9 +843,9 @@
if (error) {
LPTSTR error_text = 0;
if (FormatMessage (FORMAT_MESSAGE_ALLOCATE_BUFFER |
- FORMAT_MESSAGE_FROM_SYSTEM, NULL, error,
- MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
- error_text, 0, NULL)) {
+ FORMAT_MESSAGE_FROM_SYSTEM, NULL, error,
+ MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
+ (LPTSTR)&error_text, 0, NULL)) {
warn ("%s failed with error %d: %s\n", action, error, error_text);
LocalFree (error_text);
}
@@ -952,6 +952,8 @@
if (-1 == status.status)
return status;
+ CloseHandle (child.hThread);
+
/* Wait for the child process to terminate */
wait_code = WaitForSingleObject (child.hProcess, real_timeout);
if (WAIT_TIMEOUT != wait_code) {
@@ -966,60 +968,63 @@
status.status = -1;
status.error = warn_last_error ("Waiting for child process");
}
+
+ CloseHandle (child.hProcess);
return status;
}
/* Try to soft kill child process group if it didn't terminate, but only
on NT */
if (VER_PLATFORM_WIN32_NT == OSVer.dwPlatformId) {
- if (0 == GenerateConsoleCtrlEvent (CTRL_C_EVENT, child.dwProcessId))
- warn_last_error ("Sending child process Control-C");
- wait_code = WaitForSingleObject (child.hProcess, 1000);
- if (WAIT_TIMEOUT != wait_code) {
- if (WAIT_OBJECT_0 == wait_code) {
- if (0 == GetExitCodeProcess (child.hProcess,
- &status.status)) {
- warn_last_error ("Retrieving child process exit code");
- status.status = -1;
- }
- status.error = 1;
- }
- else {
- status.status = -1;
- status.error = warn_last_error ("Waiting for child process");
- }
- return status;
+ struct sig_event
+ {
+ DWORD signal_;
+ const char* msg_;
}
+ sig_events [] = {
+ { CTRL_C_EVENT, "Sending child process Control-C" },
+ { CTRL_BREAK_EVENT, "Sending child process Control-Break" }
+ };
- if (0 == GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT,
- child.dwProcessId))
- warn_last_error ("Sending child process Control-Break");
+ for (unsigned long i = 0;
+ i < sizeof (sig_events) / sizeof (*sig_events); ++i) {
- if (WAIT_TIMEOUT != wait_code) {
+ if (0 == GenerateConsoleCtrlEvent (sig_events [i].signal_,
+ child.dwProcessId))
+ warn_last_error (sig_events [i].msg_);
+
+ wait_code = WaitForSingleObject (child.hProcess, 1000);
+ if (WAIT_TIMEOUT == wait_code)
+ continue;
+
if (WAIT_OBJECT_0 == wait_code) {
- if (0 == GetExitCodeProcess (child.hProcess,
- &status.status)) {
+ if (0 == GetExitCodeProcess (child.hProcess, &status.status)) {
warn_last_error ("Retrieving child process exit code");
status.status = -1;
}
- status.error = 2;
+ status.error = i + 1;
}
else {
status.status = -1;
status.error = warn_last_error ("Waiting for child process");
}
+
+ CloseHandle (child.hProcess);
return status;
}
}
/* Then hard kill the child process */
if (0 == TerminateProcess (child.hProcess, 3))
warn_last_error ("Terminating child process");
+ else
+ WaitForSingleObject (child.hProcess, 1000);
if (0 == GetExitCodeProcess (child.hProcess, &status.status)) {
warn_last_error ("Retrieving child process exit code");
status.status = -1;
}
status.error = 3;
+ CloseHandle (child.hProcess);
return status;
}
#endif /* _WIN{32,64} */
