Bug #33812 for Mason: Race condition while creating object file

Tue Mar 04 19:25:55 2008 ianburrell [...] gmail.com - Ticket created

Subject:

Race condition while creating object file

There is a race condition when an object file is being created than can cause a process to get a truncated file. The cause is the second process reading the partially written file before the first process finishes writing the entire file. The timeline looks like: A: file does not exist A: open object file to write A: write first block B: file exists B: load object file A: write rest of file A: close file B sees that the file already exists and reads the first half of the file while A is still writing the rest of the file.

Tue Mar 04 19:28:09 2008 ianburrell [...] gmail.com - Correspondence added

From:

ianburrell [...] gmail.com

Here is a patch that changes HTML::Mason::Compiler::ToObject to write to a randomly-named temp file and atomically rename to the real file.

Index: lib/HTML/Mason/Compiler/ToObject.pm =================================================================== --- lib/HTML/Mason/Compiler/ToObject.pm (revision 3880) +++ lib/HTML/Mason/Compiler/ToObject.pm (working copy) @@ -113,18 +113,23 @@ ($file) = $file =~ /^(.*)/s if taint_is_on; # Untaint blindly - open my $fh, "> $file" - or system_error "Couldn't create object file $file: $!"; + my $temp_file = $file . '.' . $$ . '.' . time(); + open my $fh, "> $temp_file" + or system_error "Couldn't create temp object file $file: $!"; + $self->compile( comp_source => $source->comp_source_ref, name => $source->friendly_name, comp_class => $source->comp_class, comp_path => $source->comp_path, fh => $fh ); - close $fh - or system_error "Couldn't close object file $file: $!"; - + close $fh + or system_error "Couldn't close temp object file $temp_file: $!"; + + rename($temp_file, $file) + or system_error "Couldn't rename $temp_file to object file $file: $!"; + return \@newfiles; }

Tue Mar 11 00:51:06 2008 JSWARTZ [...] cpan.org - Correspondence added

From:

JSWARTZ [...] cpan.org

Almost, but there's still potential conflict with the tempfile name when the object directory is being shared by multiple hosts over NFS. We're doing this at my current work, so it isn't just theoretical. :) You could look at how CHI::Driver::File handles this - it's designed to work over NFS. On Tue Mar 04 19:28:09 2008, ianburrell@gmail.com wrote: Show quoted text

> Here is a patch that changes HTML::Mason::Compiler::ToObject to write to > a randomly-named temp file and atomically rename to the real file. >

Tue Mar 11 00:51:11 2008 The RT System itself - Status changed from 'new' to 'open'

Tue Mar 11 01:07:40 2008 autarch [...] urth.org - Correspondence added

Subject:	Re: [rt.cpan.org #33812] Race condition while creating object file
Date:	Tue, 11 Mar 2008 00:07:20 -0500 (CDT)
To:	JSWARTZ via RT <bug-HTML-Mason [...] rt.cpan.org>
From:	Dave Rolsky <autarch [...] urth.org>

On Tue, 11 Mar 2008, JSWARTZ via RT wrote: Show quoted text

> Almost, but there's still potential conflict with the tempfile name when > the object directory is being shared by multiple hosts over NFS. We're > doing this at my current work, so it isn't just theoretical. :)

Does File::Temp not handle this properly? -dave /*========================== VegGuide.Org Your guide to all that's veg ==========================*/

Tue Mar 11 01:39:12 2008 JSWARTZ [...] cpan.org - Correspondence added

On Tue Mar 11 01:07:40 2008, autarch@urth.org wrote: Show quoted text

> Does File::Temp not handle this properly? >

File::Temp has a bunch of warnings related to NFS. AFAICT one should not try to use File::Temp to create a temporary file in an NFS directory. The way CHI::File::Driver handles this is to generate a truly unique id to append to the final filename, but this requires Data::UUID.

Sat Aug 25 19:51:07 2012 JSWARTZ [...] cpan.org - Taken

Sat Aug 25 19:51:23 2012 JSWARTZ [...] cpan.org - Queue changed from HTML-Mason to Mason

Sat Aug 25 19:52:14 2012 JSWARTZ [...] cpan.org - Correspondence added

This bug actually applies to the latest Mason 2, so I will fix it there.