On Thu, 5 Oct 2006, Guy Rosen wrote:

> -----8<-----
>
> Index: logger.rb
> ===================================================================
> RCS file: /src/ruby/lib/logger.rb,v
> retrieving revision 1.5.2.9
> diff -r1.5.2.9 logger.rb
> 568,570d567
> <       if FileTest.exist?(age_file)
> <         raise RuntimeError.new("'#{ age_file }' already exists.")
> <       end
> 572c569,571
> <       File.rename("#{@filename}", age_file)
> ---
>>       if not FileTest.exist?(age_file)
>>         File.rename("#{@filename}", age_file)
>>       end
>

this code has a serious race condition too

   if not FileTest.exist?(age_file)
     #
     # but suddenly here it does!!
     #
     File.rename("#{@filename}", age_file)
   end

this will be especially bad on nfs and shared filesystems where the exists?
check only reads the inode and inode caching may mean a lag of 1-3 minutes
(with normal setups) before the check reflects what's on the remote server.
invalidating an inode is tricky and platform/fs dependant as well.

additionally fixing this doesn't make logger process safe: writes will
sometimes still be overlapped because logger doesn't use any sort of ipc safe
locking for writing.  if it did it would incur a massive speed penalty to boot
- esp if locking is on a shared filesystem.  also, creation of a new logfile
uses (File::WRONLY | File::APPEND | File::CREAT) which also races on any
shared fs... in summary, there's almost nothing about logger.rb which makes
it process safe ;-(


-a
-- 
in order to be effective truth must penetrate like an arrow - and that is
likely to hurt. -- wei wu wei