Skip to content

Commit eaf7e83

Browse files
max630paulusmack
authored andcommitted
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>
1 parent 1dd2960 commit eaf7e83

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

gitk

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2783,6 +2783,21 @@ proc doprogupdate {} {
27832783
}
27842784
}
27852785

2786+
proc config_check_tmp_exists {tries_left} {
2787+
global config_file_tmp
2788+
2789+
if {[file exists $config_file_tmp]} {
2790+
incr tries_left -1
2791+
if {$tries_left > 0} {
2792+
after 100 [list config_check_tmp_exists $tries_left]
2793+
} else {
2794+
error_popup "There appears to be a stale $config_file_tmp\
2795+
file, which will prevent gitk from saving its configuration on exit.\
2796+
Please remove it if it is not being used by any existing gitk process."
2797+
}
2798+
}
2799+
}
2800+
27862801
proc config_init_trace {name} {
27872802
global config_variable_changed config_variable_original
27882803

@@ -2818,11 +2833,16 @@ proc savestuff {w} {
28182833

28192834
if {$stuffsaved} return
28202835
if {![winfo viewable .]} return
2836+
set remove_tmp 0
28212837
if {[catch {
2822-
if {[file exists $config_file_tmp]} {
2823-
file delete -force $config_file_tmp
2838+
set try_count 0
2839+
while {[catch {set f [open $config_file_tmp {WRONLY CREAT EXCL}]}]} {
2840+
if {[incr try_count] > 50} {
2841+
error "Unable to write config file: $config_file_tmp exists"
2842+
}
2843+
after 100
28242844
}
2825-
set f [open $config_file_tmp w]
2845+
set remove_tmp 1
28262846
if {$::tcl_platform(platform) eq {windows}} {
28272847
file attributes $config_file_tmp -hidden true
28282848
}
@@ -2884,9 +2904,13 @@ proc savestuff {w} {
28842904
puts $f "}"
28852905
close $f
28862906
file rename -force $config_file_tmp $config_file
2907+
set remove_tmp 0
28872908
} err]} {
28882909
puts "Error saving config: $err"
28892910
}
2911+
if {$remove_tmp} {
2912+
file delete -force $config_file_tmp
2913+
}
28902914
set stuffsaved 1
28912915
}
28922916

@@ -12177,6 +12201,7 @@ catch {
1217712201
}
1217812202
source $config_file
1217912203
}
12204+
config_check_tmp_exists 50
1218012205

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

0 commit comments

Comments
 (0)