[PATCH] testbot: Better handle the database connections in case of a fork().

[Francois Gouget]

Make sure AutoInactiveDestroy is set. This will prevent a mere stray
fork()+exit() from breaking the parent's DBI connection.
Also stop using DBI::disconnect() as this bypasses the InactiveDestroy
setting, such that calling it in a child process will break the parent's
DBI connection. DBI connections are automatically closed in the
appropriate way their destructor when they get out of scope.
These two changes allow us to close the DBI connections in the child
process (which don't need them anyway), rather than breaking the
connections before the fork() and thus forcing the parent to reconnect.

Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com>
---
 testbot/bin/Engine.pl                 |  6 +++---
 testbot/lib/ObjectModel/DBIBackEnd.pm | 21 ++++++++++-----------
 testbot/lib/WineTestBot/VMs.pm        |  7 ++++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl
index d6375647..7e278ba9 100755
--- a/testbot/bin/Engine.pl
+++ b/testbot/bin/Engine.pl
@@ -309,9 +309,6 @@ sub HandleJobStatusChange($$$)
 
   if ($OldStatus eq "running" && $NewStatus ne "running")
   {
-    # Make sure the child process does not inherit the database connections
-    CloseAllDBBackEnds();
-
     my $Pid = fork;
     if (!defined $Pid)
     {
@@ -319,7 +316,10 @@ sub HandleJobStatusChange($$$)
     }
     elsif (!$Pid)
     {
+      # Clean up the child environment
+      CloseAllDBBackEnds();
       WineTestBot::Log::SetupRedirects();
+
       exec("$BinDir/${ProjectName}SendLog.pl $JobKey") or
       LogMsg "Unable to exec ${ProjectName}SendLog.pl: $!\n";
       exit(1);
diff --git a/testbot/lib/ObjectModel/DBIBackEnd.pm b/testbot/lib/ObjectModel/DBIBackEnd.pm
index 5567203b..60e68d8f 100644
--- a/testbot/lib/ObjectModel/DBIBackEnd.pm
+++ b/testbot/lib/ObjectModel/DBIBackEnd.pm
@@ -45,14 +45,11 @@ sub GetDb($)
 {
   my ($self) = @_;
 
-  if (defined $self->{Db} && !$self->{Db}->ping())
+  if (!$self->{Db} or !$self->{Db}->ping())
   {
-    # This connection no longer works, probably due to the database idle timeout
-    $self->{Db}->disconnect();
+    # We don't have a connection yet, or it no longer works (probably due to
+    # the database idle timeout).
     $self->{Db} = undef;
-  }
-  if (!defined $self->{Db})
-  {
     while (1)
     {
       # Protect this call so we can retry in case RaiseError is set
@@ -597,11 +594,9 @@ sub Close($)
 {
   my ($self) = @_;
 
-  if (defined($self->{Db}))
-  {
-    $self->{Db}->disconnect();
-    $self->{Db} = undef;
-  }
+  # The connection will be automatically closed. In a child process this
+  # will automatically do the right thing thanks to AutoInactiveDestroy.
+  $self->{Db} = undef;
 }
 
 sub UseDBIBackEnd($$$$$$)
@@ -610,9 +605,13 @@ sub UseDBIBackEnd($$$$$$)
 
   # The implementation assumes AutoCommit is on.
   $DbArgs->{AutoCommit} = 1;
+  # Make sure AutoInactiveDestroy is set so the database connection is not
+  # broken if we fork() (including if it's behind our back).
+  $DbArgs->{AutoInactiveDestroy} = 1;
 
   my $BackEnd = $class->new();
   $BackEnd->{ConnectArgs} = [$DbSource, $DbUser, $DbPassword, $DbArgs];
   AddDBBackEnd($DbSelector, $BackEnd);
 }
+
 1;
diff --git a/testbot/lib/WineTestBot/VMs.pm b/testbot/lib/WineTestBot/VMs.pm
index bb9a8328..d893e496 100644
--- a/testbot/lib/WineTestBot/VMs.pm
+++ b/testbot/lib/WineTestBot/VMs.pm
@@ -344,9 +344,6 @@ sub Run($$$$$)
   # So set ChildPid in the parent and synchronize with the child so it only
   # starts once this is done.
 
-  # Make sure the child process will use its own database connections
-  CloseAllDBBackEnds();
-
   use Fcntl;
   my ($fd_read, $fd_write);
   pipe($fd_read, $fd_write); # For synchronization
@@ -378,6 +375,9 @@ sub Run($$$$$)
     return undef;
   }
 
+  # Close the database connections
+  CloseAllDBBackEnds();
+
   # Wait for the parent to set $VM->ChildPid
   close($fd_write);
   sysread($fd_read, $fd_write, 1);
@@ -389,6 +389,7 @@ sub Run($$$$$)
   require WineTestBot::VMs;
   my $VM = WineTestBot::VMs::CreateVMs()->GetItem($self->GetKey());
   exit 1 if (($VM->ChildPid || 0) != $$);
+  # Close the database connection we just opened
   $self->GetBackEnd()->Close();
 
   require WineTestBot::Log;
-- 
2.15.1