[flock] truncate locked files explicitly to prevent race conditions

Summary:
This avoids race conditions when two processes or more try to lock a file for
writing. It could be that the process losing the race writes less than the
winner, then we get rubbish at the end of the file. Calling `ftruncate(2)` inside the critical section makes sure the
contents of the file are erased first. The harmful race was observed in
xcodebuild sometimes, as it can call infer on the same file several times in
parallel (!).

Reviewed By: jberdine

Differential Revision: D5209177

fbshipit-source-id: 744169c

infer/src/backend/PerfStats.ml

@@ -11,6 +11,8 @@
 
 open! IStd
 
+module L = Logging
+
 type perf_stats = {
   rtime : float;
   utime : float;
@@ -147,18 +149,14 @@ let register_report_at_exit file =
           let json_stats = to_json (stats ()) in
           try
             Unix.mkdir_p (Filename.dirname file);
-            let stats_oc = open_out file in
+            Utils.write_file_with_locking file ~f:(fun stats_oc ->
-            let stats_fd = Unix.descr_of_out_channel stats_oc in
+                Yojson.Basic.pretty_to_channel stats_oc json_stats;
-            ignore (Unix.flock stats_fd Unix.Flock_command.lock_exclusive);
+              );
-            Yojson.Basic.pretty_to_channel stats_oc json_stats;
-            flush stats_oc;
-            ignore (Unix.flock stats_fd Unix.Flock_command.unlock);
-            Out_channel.close stats_oc
           with exc ->
-            Format.eprintf "Info: failed to write stats to %s@\n%s@\n%s@\n%s@."
+            L.internal_error "Info: failed to write stats to %s@\n%s@\n%s@\n%s@."
               file (Exn.to_string exc) (Yojson.Basic.pretty_to_string json_stats)
               (Printexc.get_backtrace ())
         with exc ->
-          Format.eprintf "Info: failed to compute stats for %s@\n%s@\n%s@."
+          L.internal_error "Info: failed to compute stats for %s@\n%s@\n%s@."
             file (Exn.to_string exc) (Printexc.get_backtrace ())
       ) ("stats reporting in " ^ file)

infer/src/base/Logging.ml

@@ -243,7 +243,7 @@ let external_error fmt =
   log ~to_console:(not Config.quiet) external_error_file_fmts fmt
 
 let internal_error fmt =
-  log ~to_console:(not Config.developer_mode) internal_error_file_fmts fmt
+  log ~to_console:Config.developer_mode internal_error_file_fmts fmt
 
 (** Type of location in ml source: __POS__ *)
 type ml_loc = string * int * int * int

infer/src/base/Serialization.ml

@@ -97,28 +97,12 @@ let create_serializer (key : Key.t) : 'a serializer =
           (fun () -> retry_exception ~timeout:1.0 ~catch_exn ~f:read ())
           (fun () -> In_channel.close inc) in
 
-  let write_file_with_locking ?(delete=false) ~do_write fname =
-    let file_descr = Unix.openfile ~mode:[Unix.O_WRONLY; Unix.O_CREAT] fname in
-    let outc = Unix.out_channel_of_descr file_descr in
-    if Unix.flock file_descr Unix.Flock_command.lock_exclusive
-    then
-      begin
-        do_write outc;
-        flush outc;
-        ignore (Unix.flock file_descr Unix.Flock_command.unlock);
-      end;
-    Out_channel.close outc;
-    if delete
-    then
-      try Unix.unlink fname with
-      | Unix.Unix_error _ -> () in
-
   let write_to_tmp_file fname data =
     let fname_tmp = Filename.temp_file
         ~in_dir:(Filename.dirname fname) (Filename.basename fname) ".tmp" in
-    write_file_with_locking
+    Utils.write_file_with_locking
       fname_tmp
-      ~do_write:(fun outc -> Marshal.to_channel outc (key, version, data) []);
+      ~f:(fun outc -> Marshal.to_channel outc (key, version, data) []);
     fname_tmp in
 
   (* The .lock file is used to synchronize the writers.
@@ -128,10 +112,10 @@ let create_serializer (key : Key.t) : 'a serializer =
     let fname_str = DB.filename_to_string fname in
     let fname_str_lock = fname_str ^ ".lock" in
 
-    write_file_with_locking
+    Utils.write_file_with_locking
       fname_str_lock
       ~delete:true
-      ~do_write:(fun _outc ->
+      ~f:(fun _outc ->
           let (data_to_write : 'a) = match cmd with
             | Replace data ->
                 data

infer/src/base/Utils.ml

@@ -297,3 +297,21 @@ let compare_versions v1 v2 =
   let lv1  = int_list_of_version v1 in
   let lv2  = int_list_of_version v2 in
   [%compare : int list] lv1 lv2
+
+let write_file_with_locking ?(delete=false) ~f:do_write fname =
+  Unix.with_file ~mode:Unix.[O_WRONLY; O_CREAT] fname ~f:(fun file_descr ->
+      if Unix.flock file_descr Unix.Flock_command.lock_exclusive then (
+        (* make sure we're not writing over some existing, possibly longer content: some other
+           process may have snagged the file from under us between open(2) and flock(2) so passing
+           O_TRUNC to open(2) above would not be a good substitute for calling ftruncate(2)
+           below. *)
+        Unix.ftruncate file_descr ~len:Int64.zero;
+        let outc = Unix.out_channel_of_descr file_descr in
+        do_write outc;
+        flush outc;
+        ignore (Unix.flock file_descr Unix.Flock_command.unlock)
+      );
+    );
+  if delete then
+    try Unix.unlink fname with
+    | Unix.Unix_error _ -> ()

infer/src/base/Utils.mli

@@ -85,3 +85,7 @@ val suppress_stderr2 : ('a -> 'b -> 'c) -> 'a -> 'b -> 'c
     -1 if v1 is older than v2 and 0 if they are the same version.
     The versions are strings of the shape "n.m.t", the order is lexicographic. *)
 val compare_versions : string -> string -> int
+
+(** Lock file passed as argument and write into it using [f]. If [delete] then the file is unlinked
+    once this is done. *)
+val write_file_with_locking : ?delete:bool -> f:(out_channel -> unit) -> string -> unit