>Number: 3847
>Category: os-windows
>Synopsis: CGIs don't get terminated if browser aborts connection
>Confidential: no
>Severity: critical
>Priority: medium
>Responsible: apache
>State: open
>Class: sw-bug
>Submitter-Id: apache
>Arrival-Date: Fri Feb 5 12:20:00 PST 1999
>Last-Modified:
>Originator: [EMAIL PROTECTED]
>Organization:
apache
>Release: 1.3.4
>Environment:
Win32, v.1.3.4 compiled with VisualStudio 6.0
>Description:
1) browser places request to a CGI that has extended lifetime.
2) browser terminates connection (Stop or Escape is pressed)
3) the CGI doesn't get killed by Apache, and either eventually expires by
itself, or hangs around forever, which represents a problem
>How-To-Repeat:
I'm using a simple c code that just slowly dumps text into stdout forever
>Fix:
According to my observations there are several issues here that contribute
to the problem, some of them related to mod_cgi, and some of them
more general (that's why I'm filing a general Windows bug)
My understanding is that both Windows and Unix versions similarly detect that
connection is dropped, and set r->connection->aborted to 1
1) Then Windows version gets stuck in mod_cgi.c in this place:
while (ap_bgets(argsbuffer, HUGE_STRING_LEN, script_err) > 0) {
continue;
}
Basically, it attempts to read from script's stderr, and doesn't return from
the blocking ReadFile call. Unix version doesn't have this problem. Don't
know why, perhaps because it relies on SIGPIPE which doesn't work on Win32.
I circumvented this problem by using
if(!r->connection->aborted) {
while (ap_bgets(argsbuffer, HUGE_STRING_LEN, script_err) > 0) {
continue;
}
}
I'm not really suggesting to use this fix, but it allowed me to get to
other issues.
(Note that the thread can similarly get stuck in reading from script's stdout.
I didn't have this problem because my CGI generates bunch of data, yet that
might be a problem.)
2) Now, Unix version kills CGIs off in the child_main() in http_main.c through
calling
ap_clear_pool(ptrans);
which eventually gets to
free_proc_chain(a->subprocesses);
in the alloc.c which kills CGIs in the Unix version. There's bunch of Win32
code which seems to do the same job, but it's *never called*, because
a->subprocesses is always NULL.
My understanding is that ap_note_subprocess() which stores process id in the
pool in Unix version is never called in Win32 code.
To fix that, I added the ap_note_subprocess call to the ap_bspawn_child
function :
...
/* Setup the cleanup routine for the handle */
ap_note_cleanups_for_h(p, hPipeErrorRead);
/* Associate the handle with the new buffer */
ap_bpushh(*pipe_err, hPipeErrorRead);
}
// Igor's change
ap_note_subprocess(p, pid, kill_how);
}
/*
* Now that handles have been inherited, close them to be safe.
* You don't want to read or write to them accidentally, and we
* sure don't want to have a handle leak.
*/
CloseHandle(hPipeOutputWrite);
CloseHandle(hPipeInputRead);
CloseHandle(hPipeErrorWrite);
...
3) Now we do get to the body of code that terminates CGI process in the
free_proc_chain function. However, the code that calls TerminateProcess is
buggy. The reason: CGI processes are started in util_script.c via
the CreateProcess call. Note that the process ID is preserved as pid, and
the process handle is immediately closed, which is fine.
However when it comes to terminating the process, this pid, i.e. Win32 process
id is used in TerminateProcess call. This is wrong, and the process handle
should be used instead. Process handle can be retrieved using process ID
with the OpenProcess call. Several changes in the free_proc_chain did
the job for me:
...
for (p = procs; p; p = p->next) {
HANDLE pid = OpenProcess(PROCESS_ALL_ACCESS,
1, p->pid);
if(0 == pid) {
p->kill_how = kill_never;
} else
CloseHandle(pid);
}
for (p = procs; p; p = p->next) {
if (p->kill_how == kill_after_timeout) {
need_timeout = 1;
}
else if (p->kill_how == kill_always) {
// Igor's change
HANDLE pid = OpenProcess(PROCESS_ALL_ACCESS,
1, p->pid);
if(0 != pid) {
TerminateProcess(pid, 1);
CloseHandle(pid);
}
}
}
/* Sleep only if we have to... */
if (need_timeout)
sleep(3);
/* OK, the scripts we just timed out for have had a chance to clean up
* --- now, just get rid of them, and also clean up the system accounting
* goop...
*/
for (p = procs; p; p = p->next) {
if (p->kill_how == kill_after_timeout) {
// Igor's change
HANDLE pid = OpenProcess(PROCESS_ALL_ACCESS, 1, p->pid);
if(0 != pid) {
TerminateProcess(pid, 1);
CloseHandle(pid);
}
}
}
...
>Audit-Trail:
>Unformatted:
[In order for any reply to be added to the PR database, ]
[you need to include <[EMAIL PROTECTED]> in the Cc line ]
[and leave the subject line UNCHANGED. This is not done]
[automatically because of the potential for mail loops. ]
[If you do not include this Cc, your reply may be ig- ]
[nored unless you are responding to an explicit request ]
[from a developer. ]
[Reply only with text; DO NOT SEND ATTACHMENTS! ]
