Skip to content

Commit 041ef53

Browse files
committed
Make save-history extension safe for concurrent use
This makes the save-history extension check for modifications to the history file before saving it. If the history file was modified after the history was loaded and before it was saved, append only the new history lines to the history file. This can result in more lines in the history file than SAVE_HISTORY allows. However, that will be fixed the next time irb is run and the history is saved. Fixes [Bug #13654]
1 parent 65aa44e commit 041ef53

File tree

2 files changed

+51
-5
lines changed

2 files changed

+51
-5
lines changed

lib/irb/ext/save-history.rb

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ def load_history
8181
end
8282
}
8383
end
84+
@loaded_history_lines = history.size
85+
@loaded_history_mtime = File.mtime(history_file)
8486
end
8587
end
8688

@@ -105,12 +107,20 @@ def save_history
105107
raise
106108
end
107109

108-
open(history_file, "w:#{IRB.conf[:LC_MESSAGES].encoding}", 0600) do |f|
110+
if File.exist?(history_file) && @loaded_history_mtime &&
111+
File.mtime(history_file) != @loaded_history_mtime
112+
history = history[@loaded_history_lines..-1]
113+
append_history = true
114+
end
115+
116+
open(history_file, "#{append_history ? 'a' : 'w'}:#{IRB.conf[:LC_MESSAGES].encoding}", 0600) do |f|
109117
hist = history.map{ |l| l.split("\n").join("\\\n") }
110-
begin
111-
hist = hist.last(num) if hist.size > num and num > 0
112-
rescue RangeError # bignum too big to convert into `long'
113-
# Do nothing because the bignum should be treated as inifinity
118+
unless append_history
119+
begin
120+
hist = hist.last(num) if hist.size > num and num > 0
121+
rescue RangeError # bignum too big to convert into `long'
122+
# Do nothing because the bignum should be treated as inifinity
123+
end
114124
end
115125
f.puts(hist)
116126
end

test/irb/test_history.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,37 @@ def test_history_save_minus_as_infinity
127127
INPUT
128128
end
129129

130+
def test_history_concurrent_use
131+
omit "Skip Editline" if /EditLine/n.match(Readline::VERSION)
132+
IRB.conf[:SAVE_HISTORY] = 1
133+
assert_history(<<~EXPECTED_HISTORY, <<~INITIAL_HISTORY, <<~INPUT) do |history_file|
134+
exit
135+
5
136+
exit
137+
EXPECTED_HISTORY
138+
1
139+
2
140+
3
141+
4
142+
INITIAL_HISTORY
143+
5
144+
exit
145+
INPUT
146+
assert_history(<<~EXPECTED_HISTORY2, <<~INITIAL_HISTORY2, <<~INPUT2)
147+
exit
148+
EXPECTED_HISTORY2
149+
1
150+
2
151+
3
152+
4
153+
INITIAL_HISTORY2
154+
5
155+
exit
156+
INPUT2
157+
File.utime(File.atime(history_file), File.mtime(history_file) + 2, history_file)
158+
end
159+
end
160+
130161
private
131162

132163
def assert_history(expected_history, initial_irb_history, input)
@@ -143,6 +174,11 @@ def assert_history(expected_history, initial_irb_history, input)
143174
io = TestInputMethod.new
144175
io.class::HISTORY.clear
145176
io.load_history
177+
if block_given?
178+
history = io.class::HISTORY.dup
179+
yield IRB.rc_file("_history")
180+
io.class::HISTORY.replace(history)
181+
end
146182
io.class::HISTORY.concat(input.split)
147183
io.save_history
148184

0 commit comments

Comments
 (0)