fixed race condition in Mojo::Server::Prefork

mojolicious · Jan 25, 2015 · b90200a · b90200a
1 parent 5ee677d
commit b90200a
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 17 deletions.
diff --git a/Changes b/Changes
@@ -1,5 +1,6 @@
 
 5.75  2015-01-26
+  - Fixed race condition in Mojo::Server::Prefork.
 
 5.74  2015-01-25
   - Improved parser errors to be more consistent with connection errors in

diff --git a/lib/Mojo/Server/Hypnotoad.pm b/lib/Mojo/Server/Hypnotoad.pm
@@ -57,7 +57,7 @@ sub run {
   $self->configure('hypnotoad');
   weaken $self;
   $prefork->on(wait   => sub { $self->_manage });
-  $prefork->on(reap   => sub { $self->_reap(pop) });
+  $prefork->on(reap   => sub { $self->_cleanup(pop) });
   $prefork->on(finish => sub { $self->{finished} = 1 });
 
   # Testing
@@ -78,6 +78,15 @@ sub run {
   $prefork->cleanup(1)->run;
 }
 
+sub _cleanup {
+  my ($self, $pid) = @_;
+
+  # Clean up failed upgrade
+  return unless ($self->{new} || '') eq $pid;
+  $self->prefork->app->log->error('Zero downtime software upgrade failed.');
+  delete @$self{qw(new upgrade)};
+}
+
 sub _exit { say shift and exit 0 }
 
 sub _hot_deploy {
@@ -117,15 +126,6 @@ sub _manage {
   }
 }
 
-sub _reap {
-  my ($self, $pid) = @_;
-
-  # Clean up failed upgrade
-  return unless ($self->{new} || '') eq $pid;
-  $self->prefork->app->log->error('Zero downtime software upgrade failed.');
-  delete @$self{qw(new upgrade)};
-}
-
 sub _stop {
   _exit('Hypnotoad server not running.')
     unless my $pid = shift->prefork->check_pid;

diff --git a/lib/Mojo/Server/Prefork.pm b/lib/Mojo/Server/Prefork.pm
@@ -76,12 +76,7 @@ sub run {
 
   # Clean manager environment
   local $SIG{INT} = local $SIG{TERM} = sub { $self->_term };
-  local $SIG{CHLD} = sub {
-    while ((my $pid = waitpid -1, WNOHANG) > 0) {
-      $self->app->log->debug("Worker $pid stopped.")
-        if delete $self->emit(reap => $pid)->{pool}{$pid};
-    }
-  };
+  local $SIG{CHLD} = sub { $self->_reap unless $self->{forking} };
   local $SIG{QUIT} = sub { $self->_term(1) };
   local $SIG{TTIN} = sub { $self->workers($self->workers + 1) };
   local $SIG{TTOU} = sub {
@@ -117,7 +112,8 @@ sub _heartbeat {
 sub _manage {
   my $self = shift;
 
-  # Spawn more workers and check PID file
+  # Spawn more workers if necessary and check PID file
+  $self->_reap;
   if (!$self->{finished}) {
     $self->_spawn while keys %{$self->{pool}} < $self->workers;
     $self->ensure_pid_file;
@@ -156,10 +152,19 @@ sub _manage {
   }
 }
 
+sub _reap {
+  my $self = shift;
+  while ((my $pid = waitpid -1, WNOHANG) > 0) {
+    $self->app->log->debug("Worker $pid stopped.")
+      if delete $self->emit(reap => $pid)->{pool}{$pid};
+  }
+}
+
 sub _spawn {
   my $self = shift;
 
   # Manager
+  local $self->{forking} = 1;
   die "Can't fork: $!" unless defined(my $pid = fork);
   return $self->emit(spawn => $pid)->{pool}{$pid} = {time => steady_time}
     if $pid;