[1/3] testbot/lib: Add utility functions to atomically create new files.
Francois Gouget
fgouget at codeweavers.com
Mon Jun 16 09:10:48 CDT 2014
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
--
2.0.0
More information about the wine-patches
mailing list