Merging postreleasefix/90 to release/90

* postreleasefix/90:
  subversion update
  add some extra fork-failure safeguards, checks and tests

modules/Bio/EnsEMBL/VEP/Constants.pm

@@ -53,7 +53,7 @@ use warnings;
 use base qw(Exporter);
 
 our $VEP_VERSION     = 90;
-our $VEP_SUB_VERSION = 4;
+our $VEP_SUB_VERSION = 5;
 
 our @EXPORT_OK = qw(
   @FLAG_FIELDS

modules/Bio/EnsEMBL/VEP/Runner.pm

@@ -456,7 +456,7 @@ sub _forked_buffer_to_output {
   my $minForkSize = 50;
   my $maxForkSize = int($buffer_size / (2 * $fork_number)) || 1;
   my $active_forks = 0;
-  my (@pids, %by_pid);
+  my (@pids, %by_pid, %children_reported_back);
   my $sel = IO::Select->new;
 
   # loop while variants in @$vfs or forks running
@@ -491,14 +491,17 @@ sub _forked_buffer_to_output {
         elsif($pid) {
           push @pids, $pid;
           $active_forks++;
+
+          # for testing
+          kill $self->{_kill_child}, $pid if $self->{_kill_child};
         }
         elsif($pid == 0) {
           $self->_forked_process($buffer, \@tmp, $parent, $output_as_hash);
         }
       }
     }
 
-    # read child input
+    # read child output
     while(my @ready = $sel->can_read()) {
       my $no_read = 1;
 
@@ -538,20 +541,43 @@ sub _forked_buffer_to_output {
           merge_hashes($cache, $fork_vep_cache);
         }
 
+        # report this child as done
+        $children_reported_back{$data->{pid}} = 1;
+
         # finish up
         $sel->remove($fh);
         $fh->close;
         $active_forks--;
       }
 
       # read-through detected, DIE
-      throw("\nERROR: Forked process(es) died\n") if $no_read;
+      throw("\nERROR: Forked process(es) died: read-through of cross-process communication detected\n") if $no_read;
 
       last if $active_forks < $fork_number;
     }
   }
 
-  waitpid($_, 0) for @pids;
+  for my $pid(@pids) {
+    if(waitpid($pid, 0) > 0) {
+      my ($rc, $sig, $core) = ($? >> 8, $? & 127, $? & 128);
+      if($core){
+        throw("\nERROR: Forked process PID $pid dumped core\n");
+      }
+      elsif($sig == 9) {
+        throw("\nERROR: Forked process PID $pid was killed\n");
+      }
+      elsif($rc != 0) {
+        throw("\nERROR: Forked process PID $pid returned non-zero return code $rc\n");
+      }
+    }
+    else {
+      throw("\nERROR: Forked process PID $pid disappeared\n");
+    }
+  }
+
+  # check any children didn't report back
+  my @unclean = grep {!$children_reported_back{$_}} @pids;
+  throw("\nERROR: Forked process(es) ".join(", ", @unclean)." did not finish cleanly") if @unclean;
 
   # sort data by dispatched PID order
   my @return = map {@{$by_pid{$_} || []}} @pids;
@@ -592,6 +618,14 @@ sub _forked_process {
   my $parent = shift;
   my $output_as_hash = shift;
 
+  # for testing
+  kill $self->{_kill_self}, $$ if $self->{_kill_self};
+
+  # simulate a memory leak
+  # my $loop = 1;
+  # my $mem_leak = sub { my ($a, $b); $a = \$b; $b = \$a; print STDERR "$$ ".($loop / 1e6)."\n" if ++$loop % 1e6 == 0};
+  # &$mem_leak() while(1);
+
   # redirect and capture STDERR
   $self->config->{warning_fh} = *STDERR;
   close STDERR;

t/Runner.t

@@ -931,6 +931,26 @@ $runner->{_test_die} = 1;
 
 throws_ok {$runner->next_output_line} qr/TEST DIE/, 'fork - test die';
 
+
+$runner = Bio::EnsEMBL::VEP::Runner->new({
+  %$cfg_hash,
+  offline => 1,
+  fork => 1,
+});
+$runner->param('warning_file', 'STDERR');
+$runner->{_kill_child} = 'KILL';
+throws_ok {$runner->next_output_line} qr/Forked process.+ died.+read-through/, 'fork - kill child';
+
+
+$runner = Bio::EnsEMBL::VEP::Runner->new({
+  %$cfg_hash,
+  offline => 1,
+  fork => 1,
+});
+$runner->param('warning_file', 'STDERR');
+$runner->{_kill_self} = 'KILL';
+throws_ok {$runner->next_output_line} qr/Forked process.+ died.+read-through/, 'fork - child kill self';
+
 # restore STDERR
 open(STDERR, ">&SAVE") or die "Can't restore STDERR\n";