Randy Kobes wrote:There are also "ExecutablePath" and "CommandLine" entries in the hash returned by W::P::I which could be used to check that it is *our* Apache server.
Given that, it seems to me better to use
Win32::Process::Info to get the name of the process, and
kill that if the name is 'Apache.exe' (of course, this
isn't foolproof, as it may be a legitimate Apache process
started from somewhere else, but one has to draw a line ..).
Stas, would you object to making Win32::Process::Info a
requirement on Win32 for Apache-Test? I'll supply ppm
versions of it (and also the required Win32::API), so
for most users it should be easy to install.
Can we try first to see if we can get away without making A-T depend on Win32::Process::Info? If anything else fails, than it's OK I guess.
How about the following approaches:
1) after attempting to kill httpd, remove the pid file no matter what. If the process wasn't killed, so be it, the user will have to kill it manually. (he will know that since the port won't be available)
I agree it is undesirable to have A::T depend on W::P::I unless necessary, and I think we can get pretty close to what we want without it.
How about the attached patch, which is a slight refinement of Randy's first patch. There are three refinements here:
1. I've changed the warning in the "else" part of the "if kill(0, $pid)" in stop() to simply say that the server has gone away, which is what that would mean -- there is no Windows error to retrieve at that point.
2. I've removed the "require Win32;" lines -- FormatMessage() and GetLastError() are both listed as "[CORE]" in the Win32 manpage, meaning that "require Win32;" is not required ;)
3. Most importantly, I've moved the clean-up of stale PID files in start() to the top of the sub. In Randy's patch, start() would call stop() early which would possibly work on a stale PID file, which is what we're trying to prevent, and then clean-up the stale file later (too late). We can't reliably do the stop() on Win32 when just starting up since we'd *have* to be using an *old* PID file at that point. (Such stale files may still exist despite our best efforts to delete them when stopping, e.g. if the server *crashed*.) Therefore, start() now doesn't bother with a stop() on Win32.
There are still potential problems with this -- e.g. if the server crashes during a smoke run then the parent smoke process that kicked off the server will still attempt to stop it (using t/TEST -stop), and will be using the PID file left behind by that crash. However, a long smoke run would most likely be run on a machine that is not otherwise in use, so the chance of that PID having been re-used is less than usual.
Likewise, if a user interactively runs "t/TEST -stop" themselves then the PID file used may be stale. But then you could argue that such interactive use suggests the user knows what he is doing. Normally people just run "nmake test", "perl t/TEST" or "perl t/SMOKE", or else they're delving deeper into something and have a better grasp of what is going on.
I think I'm happy enough with this. You could conceivably add some code to make use of W::P::I *if it is available* to handle the above problems better, but it may not be worth it.
2) try to use 'httpd -k stop' on httpd 2.0, which should do a clean stop, removing the httpd.pid file. That won't work on 1.3, where we may be able to use (1). I remember you have mentioned that Apache.exe -k doesn't work?
Indeed - "Apache.exe -k" only operates on Apache servers that have been installed as Windows Services, not on those kicked off in a shell, and not all users will have the appropriate permissions to install/start/stop/remove Services.
- Steve
diff -ruN modperl-2.0.orig/Apache-Test/lib/Apache/TestServer.pm
modperl-2.0/Apache-Test/lib/Apache/TestServer.pm
--- modperl-2.0.orig/Apache-Test/lib/Apache/TestServer.pm 2003-10-10
05:05:56.000000000 +0100
+++ modperl-2.0/Apache-Test/lib/Apache/TestServer.pm 2003-10-15 10:41:23.647485900
+0100
@@ -12,6 +12,7 @@
use Apache::TestRequest ();
use constant COLOR => Apache::TestConfig::COLOR;
+use constant WIN32 => Apache::TestConfig::WIN32;
my $CTRL_M = COLOR ? "\r" : "\n";
@@ -313,19 +314,21 @@
my $self = shift;
my $aborted = shift;
- if (Apache::TestConfig::WIN32) {
- if ($self->{config}->{win32obj}) {
- $self->{config}->{win32obj}->Kill(0);
- warning "server $self->{name} shutdown";
- return 1;
+ if (WIN32) {
+ require Win32::Process;
+ my $obj = $self->{config}->{win32obj};
+ if (my $pid = $obj ? $obj->GetProcessID : $self->pid) {
+ if (kill(0, $pid)) {
+ Win32::Process::KillProcess($pid, 0);
+ warning "server $self->{name} shutdown";
+ }
+ else {
+ warning "Could not shutdown $self->{name}: " .
+ "server has gone away\n";
+ }
}
- else {
- require Win32::Process;
- my $pid = $self->pid;
- Win32::Process::KillProcess($pid, 0);
- warning "server $self->{name} shutdown";
- return 1;
- }
+ unlink $self->pid_file if (-f $self->pid_file);
+ return 1;
}
my $pid = 0;
@@ -415,7 +418,22 @@
sub start {
my $self = shift;
- my $old_pid = $self->stop;
+ my $old_pid = 0;
+ if (WIN32) {
+ # Stale PID files (e.g. left behind from a previous test run
+ # that crashed) cannot be trusted on Windows because PID's are
+ # re-used too frequently, so just remove it. If there is an old
+ # server still running then the attempt to start a new one below
+ # will simply fail because the port will be unavailable.
+ if (-f $self->pid_file) {
+ warn "Removing old PID file -- " .
+ "Unclean shutdown of previous test run?\n";
+ unlink $self->pid_file;
+ }
+ }
+ else {
+ $old_pid = $self->stop;
+ }
my $cmd = $self->start_cmd;
my $config = $self->{config};
my $vars = $config->{vars};
@@ -436,7 +454,7 @@
print "$cmd\n";
my $old_sig;
- if (Apache::TestConfig::WIN32) {
+ if (WIN32) {
#make sure only 1 process is started for win32
#else Kill will only shutdown the parent
my $one_process = $self->version_of(\%one_process);
@@ -450,7 +468,11 @@
"$cmd $one_process",
1,
Win32::Process::NORMAL_PRIORITY_CLASS(),
- '.') || die Win32::Process::ErrorReport();
+ '.');
+ unless ($obj) {
+ die "Could not start the server: " .
+ Win32::FormatMessage(Win32::GetLastError());
+ }
$config->{win32obj} = $obj;
}
else {
diff -ruN modperl-2.0.orig/Apache-Test/lib/Apache/TestSmoke.pm
modperl-2.0/Apache-Test/lib/Apache/TestSmoke.pm
--- modperl-2.0.orig/Apache-Test/lib/Apache/TestSmoke.pm 2003-10-10
05:05:56.000000000 +0100
+++ modperl-2.0/Apache-Test/lib/Apache/TestSmoke.pm 2003-10-15 10:13:09.060440700
+0100
@@ -717,6 +717,7 @@
}
sub kill_proc {
+ return if Apache::TestConfig::WIN32;
my($self) = @_;
# a hack
@@ -724,8 +725,9 @@
my $file = catfile $t_logs, "httpd.pid";
return unless -f $file;
- my $pid = `cat $file`;
- chomp $pid;
+ my $fh = Symbol::gensym();
+ open $fh, $file or return;
+ chomp(my $pid = <$fh> || '');
return unless $pid;
kill SIGINT => $pid;--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
