Re: Avoiding race condition with sysopen

The logic is OK but you have created a possible infinite loop. sysopen may fail with the flags you have if the file exists but it may fail for other reasons ie Permissions. If you have a perms problem you code will loop forever burning 100% CPU in all liklihood. Here is a tempfile function we use that may be the sort of thing you are after. It returns the locked $fh and $filename (for unlinking or whatever) if it succeeds and limits the number of retries to less than infinity :-) Note File::Temp is a well tested solution and offers features not present in this function.

use Fcntl; # need constants O_CREAT | O_EXCL | O_RDWR

sub get_tempfile {
    my ( $dir, $prefix, $max_tries ) = @_;
    $dir       ||= '/tmp';
    $prefix    ||= 'temp';
    $max_tries ||= 10;
    my @CHARS = ( 'A' .. 'Z', 'a' .. 'z', 0 .. 9, '_' );
    ui_system_error( "$dir is not a directory" ) unless -d $dir;
    ui_system_error( "$dir is not writable" )    unless -w $dir;
    $dir =~ s!/$!!;
    my $fh;
    for ( 0 .. $max_tries ) {
        my $name = sprintf "$prefix-%d-", time();
        $name .= $CHARS[ int( rand( $#CHARS ) ) ] for 1..10;
        my $path = "$dir/$name";
      return ($fh, $name) if sysopen($fh, $path, O_CREAT | O_EXCL | O_
+RDWR, 0600);
    }
    ui_system_error( "Could not create unique filename after $max_trie
+s" );
}
[download]

cheers

tachyon

Comment on Re: Avoiding race condition with sysopen Download Code

Replies are listed 'Best First'.
Re: Avoiding race condition with sysopen by Abigail-II (Bishop) on Mar 04, 2004 at 23:54 UTC
Any reason you aren't using `File::Temp`? Abigail	[reply]
Re: Re: Avoiding race condition with sysopen by tachyon (Chancellor) on Mar 05, 2004 at 01:10 UTC
Several actually. The code is essentially the same as File::Temp uses but more compact as it just does what we want. File::Temp is 1800 lines, has lots of features and is cross platform. I just need the 15 lines I have running on one platform. The temp file names we use also include $$ (I edited that out) as we like to know which processes are sh!ting in the tmp dir so we know who to blame ;-) We handle all error conditions via our standard handlers without having to resort to catching with eval. We have a test suite for this that makes sure all is well with this bit of code so should there be an issue we would find it in normal testing. Yes we could just test File::Temp ourselves but testing a single short function is far easier. And finally because we crossed swords on usenet maybe 4 years ago on the best way to make temp files so I looked into File::Temp (on your advice) and extracted the core features into that snippet. While using modules is often a good idea we have found that Perl tends to leak memory when run persistently and hard. Less code ie just what you need has two effects. Perl leaks less and the leaks are easier to fix. LWP and URI as well as our own OO modules are problematic in this regard in our experience. It also uses less resources mainly memory and runs faster. I think a strong case could be made that at 1800 lines File::Temp is bloatware. cheers tachyon	[reply]
Re: Re: Avoiding race condition with sysopen by revdiablo (Prior) on Mar 05, 2004 at 02:54 UTC
Thanks, this is exactly the kind of thing I was looking for. I'll likely end up stealing your sanity checks. :-) As for your reply to Abigail-II's note, I am avoiding File::Temp for similar reasons. Even adding the extra checking you have here, my code will do far less and have a much smaller intended scope than File::Temp. Furthermore, I am not making temporary files, so using a module designed to do so seems to me kind of like using `$#array+1` when I want `scalar @array`.	[reply] [d/l] [select]