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@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;