gitk: Synchronize config file writes

If several gitk instances are closed simultaneously, the savestuff
procedure can run at the same time, resulting in a conflict which may
cause losing of some of the instance's changes, failing the saving
operation or even corrupting the configuration file. This can happen,
for example, at user session closing, or at group closing of all
instances of an application which is possible in some desktop
environments.

To avoid this, make sure that only one saving operation is in
progress.  It is guarded by existence of the $config_file_tmp
file. Creating the file and moving it to $config_file are both atomic
operations, so it should be reliable.

Reading does not need to be syncronized, because moving is an atomic
operation, and the $config_file always refers to a full and correct file.
But, if there is a stale $config_file_tmp file, report it at gitk start.
If such file is detected when saving, just report it abort the save, as
for other errors in saving.

Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Paul Mackerras <paulus@samba.org>

diff --git a/gitk b/gitk
index 26a7db4c8c27415738ce9ced7754563cf3f2d28c..0325ab0ca2b60876db2b5fc3209e3fdcc77fe5a0 100755 (executable)
--- a/gitk
+++ b/gitk
@@ -2783,6 +2783,21 @@ proc doprogupdate {} {
     }
 }
 
     }
 }
 

+proc config_check_tmp_exists {tries_left} {
+    global config_file_tmp
+
+    if {[file exists $config_file_tmp]} {
+       incr tries_left -1
+       if {$tries_left > 0} {
+           after 100 [list config_check_tmp_exists $tries_left]
+       } else {
+           error_popup "There appears to be a stale $config_file_tmp\
+ file, which will prevent gitk from saving its configuration on exit.\
+ Please remove it if it is not being used by any existing gitk process."
+       }
+    }
+}
+

 proc config_init_trace {name} {
     global config_variable_changed config_variable_original
 
 proc config_init_trace {name} {
     global config_variable_changed config_variable_original
 


@@ -2818,11 +2833,16 @@ proc savestuff {w} {
 
     if {$stuffsaved} return
     if {![winfo viewable .]} return
 
     if {$stuffsaved} return
     if {![winfo viewable .]} return

+    set remove_tmp 0

     if {[catch {
     if {[catch {

-       if {[file exists $config_file_tmp]} {
-           file delete -force $config_file_tmp
+       set try_count 0
+       while {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {
+           if {[incr try_count] > 50} {
+               error "Unable to write config file: $config_file_tmp exists"
+           }
+           after 100

        }
        }

-       set f [open $config_file_tmp w]
+       set remove_tmp 1

        if {$::tcl_platform(platform) eq {windows}} {
            file attributes $config_file_tmp -hidden true
        }
        if {$::tcl_platform(platform) eq {windows}} {
            file attributes $config_file_tmp -hidden true
        }


@@ -2884,9 +2904,13 @@ proc savestuff {w} {
        puts $f "}"
        close $f
        file rename -force $config_file_tmp $config_file
        puts $f "}"
        close $f
        file rename -force $config_file_tmp $config_file

+       set remove_tmp 0

     } err]} {
         puts "Error saving config: $err"
     }
     } err]} {
         puts "Error saving config: $err"
     }

+    if {$remove_tmp} {
+       file delete -force $config_file_tmp
+    }

     set stuffsaved 1
 }
 
     set stuffsaved 1
 }
 


@@ -12177,6 +12201,7 @@ catch {
     }
     source $config_file
 }
     }
     source $config_file
 }

+config_check_tmp_exists 50

 
 set config_variables {
     mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth
 
 set config_variables {
     mainfont textfont uifont tabstop findmergefiles maxgraphpct maxwidth