Module: tools Branch: master Commit: e803a3fa8270cb2eff0f2be5844ee263d66b62db URL: http://source.winehq.org/git/tools.git/?a=commit;h=e803a3fa8270cb2eff0f2be58...
Author: Francois Gouget fgouget@codeweavers.com Date: Mon Jun 16 16:10:48 2014 +0200
testbot/lib: Add utility functions to atomically create new files.
These ensure there are no race conditions with other processes. They also simplify the calling code.
---
testbot/bin/CheckForWinetestUpdate.pl | 36 ++++++++++++--------------- testbot/bin/WinePatchesMLSubmit.pl | 14 ++++++----- testbot/lib/WineTestBot/PendingPatchSets.pm | 27 ++++++-------------- testbot/lib/WineTestBot/Utils.pm | 28 ++++++++++++++++++++- testbot/web/Submit.pl | 17 ++++++------- 5 files changed, 65 insertions(+), 57 deletions(-)
diff --git a/testbot/bin/CheckForWinetestUpdate.pl b/testbot/bin/CheckForWinetestUpdate.pl index 17b75d0..3bedd88 100755 --- a/testbot/bin/CheckForWinetestUpdate.pl +++ b/testbot/bin/CheckForWinetestUpdate.pl @@ -38,6 +38,7 @@ sub BEGIN }
use Fcntl; +use File::Basename; use File::Compare; use File::Copy; use LWP::UserAgent; @@ -65,17 +66,20 @@ sub AddJob($$$) my ($BaseJob, $LatestBaseName, $Bits) = @_;
# Create a copy in staging so it can then be moved into the job directory - my $FileNameRandomPart = GenerateRandomString(32); - while (-e "$DataDir/staging/${FileNameRandomPart}_$LatestBaseName") + my ($fh, $StagingFileName) = OpenNewFile("$DataDir/staging", "_$LatestBaseName"); + if (!$fh) { - $FileNameRandomPart = GenerateRandomString(32); + LogMsg "Could not create the staging file: $!\n"; + return 0; } - my $StagingFileName = "$DataDir/staging/${FileNameRandomPart}_$LatestBaseName"; - if (!copy("$DataDir/latest/$LatestBaseName", $StagingFileName)) + if (!copy("$DataDir/latest/$LatestBaseName", $fh)) { LogMsg "Could not copy '$DataDir/latest/$LatestBaseName' to '$StagingFileName': $!\n"; + close($fh); + unlink($StagingFileName); return 0; } + close($fh);
# First create a new job my $Jobs = CreateJobs(); @@ -91,7 +95,7 @@ sub AddJob($$$) my $NewStep = $Steps->Add(); my $BitsSuffix = ($Bits == 64 ? "64" : ""); $NewStep->Type("suite"); - $NewStep->FileName("${FileNameRandomPart}_$LatestBaseName"); + $NewStep->FileName(basename($StagingFileName)); $NewStep->FileType($Bits == 64 ? "exe64" : "exe32"); $NewStep->InStaging(1);
@@ -219,24 +223,16 @@ if ($Response->code != RC_OK) # put it in the jobs directory tree. umask 002; mkdir "$DataDir/staging"; -my $FileNameRandomPart = GenerateRandomString(32); -while (-e "$DataDir/staging/${FileNameRandomPart}_winetest${BitsSuffix}-latest.exe") +my ($fh, $StagingFileName) = OpenNewFile("$DataDir/staging", "_$LatestBaseName"); +if (!$fh) { - $FileNameRandomPart = GenerateRandomString(32); + LogMsg "Could not create staging file: $!\n"; + exit 1; } -my $StagingFileName = "$DataDir/staging/${FileNameRandomPart}_winetest${BitsSuffix}-latest.exe"; +print $fh $Response->decoded_content(); +close($fh);
my $NewFile = 1; -if (open(my $fh, ">", $StagingFileName)) -{ - print $fh $Response->decoded_content(); - close($fh); -} -else -{ - LogMsg "Could not create staging file '$StagingFileName': $!\n"; - exit 1; -} if (-r $LatestFileName) { $NewFile = compare($StagingFileName, $LatestFileName) != 0; diff --git a/testbot/bin/WinePatchesMLSubmit.pl b/testbot/bin/WinePatchesMLSubmit.pl index e904bbc..359e4c0 100755 --- a/testbot/bin/WinePatchesMLSubmit.pl +++ b/testbot/bin/WinePatchesMLSubmit.pl @@ -44,17 +44,19 @@ use WineTestBot::Engine::Notify; use WineTestBot::Log;
# Store the message in the staging dir -my $FileName = GenerateRandomString(32) . "_wine-patches"; -while (-e ("$DataDir/staging/$FileName")) +my ($fh, $FileName) = OpenNewFile("$DataDir/staging", "_wine-patches"); +if (!$fh) { - $FileName = GenerateRandomString(32); + LogMsg "Could not create a staging file: $!\n"; + exit 1; } -if (!copy(*STDIN, "$DataDir/staging/$FileName")) +if (!copy(*STDIN, $fh)) { - LogMsg "Unable to copy the email to '$FileName': $!\n"; + LogMsg "Could not copy the email to '$FileName': $!\n"; + close($fh); exit 1; } - +close($fh);
# Notify the Engine of the new message my $ErrMessage = WinePatchMLSubmission(); diff --git a/testbot/lib/WineTestBot/PendingPatchSets.pm b/testbot/lib/WineTestBot/PendingPatchSets.pm index 4f908b5..7a837db 100644 --- a/testbot/lib/WineTestBot/PendingPatchSets.pm +++ b/testbot/lib/WineTestBot/PendingPatchSets.pm @@ -107,34 +107,21 @@ sub SubmitSubset my $self = shift; my ($MaxPart, $FinalPatch) = @_;
- my $CombinedFileName = "$DataDir/staging/" . GenerateRandomString(32) . - "_patch"; - while (-e $CombinedFileName) - { - $CombinedFileName = "$DataDir/staging/" . GenerateRandomString(32) . - "_patch"; - } - - if (! open(COMBINED, ">$CombinedFileName")) - { - return "Can't create combined patch file"; - } + my ($CombinedFile, $CombinedFileName) = OpenNewFile("$DataDir/staging", "_patch"); + return "Could not create a combined patch file: $!" if (!$CombinedFile);
my $Parts = $self->Parts; for (my $PartNo = 1; $PartNo <= $MaxPart; $PartNo++) { my $Part = $Parts->GetItem($PartNo); - if (defined($Part)) + if (defined $Part and + open(my $PartFile, "<" , "$DataDir/patches/" . $Part->Patch->Id)) { - if (open(PART, "<$DataDir/patches/" . $Part->Patch->Id)) - { - print COMBINED <PART>; - close(PART); - } + map { print $CombinedFile $_; } <$PartFile>; + close($PartFile); } } - - close(COMBINED); + close($CombinedFile);
my $ErrMessage = $FinalPatch->Submit($CombinedFileName, 1); unlink($CombinedFileName); diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 1084abd..f05c7dc 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -24,6 +24,8 @@ WineTestBot::Utils - Utility functions
=cut
+use Fcntl; + use WineTestBot::Config;
use vars qw (@ISA @EXPORT); @@ -31,7 +33,7 @@ use vars qw (@ISA @EXPORT); require Exporter; @ISA = qw(Exporter); @EXPORT = qw(&MakeSecureURL &SecureConnection &GenerateRandomString - &BuildEMailRecipient); + &OpenNewFile &CreateNewFile &BuildEMailRecipient);
sub MakeSecureURL($) { @@ -65,6 +67,30 @@ sub GenerateRandomString($) return substr($RandomString, 0, $Len); }
+sub OpenNewFile($$) +{ + my ($Dir, $Suffix) = @_; + + while (1) + { + my $fh; + my $FileName = "$Dir/" . GenerateRandomString(32) . $Suffix; + return ($fh, $FileName) if (sysopen($fh, $FileName, O_CREAT | O_EXCL | O_WRONLY)); + + # This is not an error that will be fixed by trying a different filename + return (undef, undef) if (!$!{EEXIST}); + } +} + +sub CreateNewFile($$) +{ + my ($Dir, $Suffix) = @_; + + my ($fh, $FileName) = OpenNewFile($Dir, $Suffix); + close($fh) if ($fh); + return $FileName; +} + sub DateTimeToString($) { my ($Time) = @_; diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index c760446..823be4b 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -792,20 +792,17 @@ sub OnSubmit($) # it so it does not get overwritten if the user submits another one before # the Engine gets around to doing so. my $BaseName = $self->GetParam("FileName"); - my $FileNameRandomPart = GenerateRandomString(32); - while (-e "$DataDir/staging/${FileNameRandomPart}_$BaseName") - { - $FileNameRandomPart = GenerateRandomString(32); - } - my $StagingFileName = "${FileNameRandomPart}_$BaseName"; + my $StagingFileName = CreateNewFile("$DataDir/staging", "_$BaseName");
my $TmpStagingFullPath = $self->GetTmpStagingFullPath($BaseName); - if (!rename($TmpStagingFullPath, "$DataDir/staging/$StagingFileName")) + if ($StagingFileName and !rename($TmpStagingFullPath, $StagingFileName)) { - # Use the existing staging file and hope for the best. - $self->{ErrMessage} = "Could not rename '$TmpStagingFullPath' to '$DataDir/staging/$StagingFileName': $!"; - $StagingFileName = basename($TmpStagingFullPath); + $self->{ErrMessage} = "Could not rename '$TmpStagingFullPath' to '$StagingFileName': $!\n"; + unlink($StagingFileName); + $StagingFileName = undef; } + # If needed fall back to the existing staging file and hope for the best. + $StagingFileName = basename($StagingFileName || $TmpStagingFullPath);
# See also Patches::Submit() in lib/WineTestBot/Patches.pm