Fix Tocttou race condition in File patch

[spjsschl]

There is a time of check to time of use race condition, that triggers from time to time for us. This patch fixes it. The problem is when test on line 876 passes and then the file gets removed from an outside source line 878 raises. In that case the original file is gone and can no longer be used, so return false.

[zenspider]

I'm not sure this patch is correct... You're talking about a race between test ?f and File.rename but your patch is putting the rescue at the method level, bypassing the File.open + yield, right? As such, the content won't actually get written to disk and everything will fail downstream. I think the rescue should go on the rename to allow the File.open to finish.

How does this look?

@@ -874,8 +874,12 @@
     # move previous version to the side if it exists
     renamed = false
     if test ?f, path then
-      renamed = true
-      File.rename path, path + ".old"
+      begin
+        File.rename path, path + ".old"
+        renamed = true
+      rescue SystemCallError
+        # do nothing
+      end
     end

     File.open(path, "w") do |io|

[spjsschl]

You are right with your observation. This patch just works in our system, since the file is compiled successfully the first time and the compilation result found later. So skipping the File.open and yield is not a problem. My fix let's the race condition happen and recovers in a way that worked for us, but might not work for everyone. Your suggested fix would work for everyone so it is pbly the way to go.

[zenspider]

OK. I've applied my version of the fix (and modernized that test call, gah!). It'll go out in the next scheduled release. Thank you!