Skip to content

Commit a446486

Browse files
Fix silent hang restoring CPU turbo at exit
The at_exit turbo/boost restore ran 'sudo -S' with stderr routed to /dev/null, so on long runs (past sudo's credential cache) it blocked waiting for a password whose prompt was invisible -- indistinguishable from a hang, and it left turbo disabled. Keep the sudo timestamp warm with a background 'sudo -n -v' thread, and make the restore use 'sudo -n' so it fails fast and prints the manual command instead of blocking. Applies to both Intel and AMD.
1 parent 55c1d3a commit a446486

2 files changed

Lines changed: 97 additions & 16 deletions

File tree

lib/cpu_config.rb

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,24 @@ def maximize_frequency
5252
def check_pstate(turbo:)
5353
# Override in subclasses
5454
end
55+
56+
def keep_sudo_alive
57+
@sudo_keepalive ||= Thread.new { sudo_keepalive_loop }
58+
end
59+
60+
def sudo_keepalive_loop
61+
loop do
62+
sleep 50 # comfortably under sudo's default timestamp_timeout (15 min)
63+
break unless system("sudo", "-n", "-v", out: File::NULL, err: File::NULL)
64+
end
65+
end
66+
67+
def sudo_restore(manual_hint, shell_cmd)
68+
return if system("sudo", "-n", "sh", "-c", shell_cmd, err: File::NULL)
69+
70+
warn "\nCould not restore CPU setting automatically (sudo credentials expired)."
71+
warn "Run this manually:\n #{manual_hint}"
72+
end
5573
end
5674

5775
# Intel CPU configuration
@@ -67,7 +85,9 @@ class IntelCPUConfig < CPUConfig
6785
def disable_turbo_boost
6886
# sudo requires the flag '-S' in order to take input from stdin
6987
BenchmarkRunner.check_call("sudo -S sh -c 'echo #{TURBO_DISABLED_VALUE} > #{NO_TURBO_PATH}'")
70-
at_exit { BenchmarkRunner.check_call("sudo -S sh -c 'echo 0 > #{NO_TURBO_PATH}'", quiet: true) }
88+
keep_sudo_alive
89+
restore = "sudo sh -c 'echo 0 > #{NO_TURBO_PATH}'"
90+
at_exit { sudo_restore(restore, "echo 0 > #{NO_TURBO_PATH}") }
7191
end
7292

7393
def maximize_frequency
@@ -114,7 +134,9 @@ class AMDCPUConfig < CPUConfig
114134
def disable_turbo_boost
115135
# sudo requires the flag '-S' in order to take input from stdin
116136
BenchmarkRunner.check_call("sudo -S sh -c 'echo #{TURBO_DISABLED_VALUE} > #{BOOST_PATH}'")
117-
at_exit { BenchmarkRunner.check_call("sudo -S sh -c 'echo #{TURBO_ENABLED_VALUE} > #{BOOST_PATH}'", quiet: true) }
137+
keep_sudo_alive
138+
restore = "sudo sh -c 'echo #{TURBO_ENABLED_VALUE} > #{BOOST_PATH}'"
139+
at_exit { sudo_restore(restore, "echo #{TURBO_ENABLED_VALUE} > #{BOOST_PATH}") }
118140
end
119141

120142
def maximize_frequency

test/cpu_config_test.rb

Lines changed: 73 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@
121121
at_exit_block = nil
122122
exit_called = false
123123
read_count = 0
124+
cpu_config = nil
124125

125126
File.stub :exist?, ->(path) { path.include?('intel_pstate') } do
126127
cpu_config = IntelCPUConfig.new
@@ -140,7 +141,9 @@
140141
end
141142
} do
142143
capture_io do
143-
cpu_config.configure_for_benchmarking(turbo: false)
144+
cpu_config.stub :keep_sudo_alive, -> {} do
145+
cpu_config.configure_for_benchmarking(turbo: false)
146+
end
144147
end
145148
assert_operator call_count, :>, 0, "Should call check_call to configure Intel CPU"
146149
assert at_exit_called, "Should register at_exit handler to restore CPU settings"
@@ -152,15 +155,14 @@
152155
end
153156
end
154157

155-
# Verify at_exit block restores Intel turbo settings
156-
cleanup_commands = []
157-
BenchmarkRunner.stub :check_call, ->(cmd, **opts) { cleanup_commands << { cmd: cmd, opts: opts } } do
158+
# Verify at_exit block restores Intel turbo settings non-interactively.
159+
# `sudo -n` is what prevents the silent hang when credentials have lapsed.
160+
restore_args = nil
161+
cpu_config.stub :system, ->(*args, **_opts) { restore_args = args; true } do
158162
at_exit_block.call
159163
end
160164

161-
assert_equal 1, cleanup_commands.length, "at_exit block should call check_call once"
162-
assert_equal "sudo -S sh -c 'echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo'", cleanup_commands[0][:cmd]
163-
assert_equal({ quiet: true }, cleanup_commands[0][:opts])
165+
assert_equal ["sudo", "-n", "sh", "-c", "echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo"], restore_args
164166
end
165167

166168
it 'exits when Intel turbo is not disabled and turbo flag is false' do
@@ -261,6 +263,7 @@
261263
at_exit_block = nil
262264
exit_called = false
263265
read_count = 0
266+
cpu_config = nil
264267

265268
File.stub :exist?, ->(path) { path.include?('cpufreq/boost') } do
266269
cpu_config = AMDCPUConfig.new
@@ -278,7 +281,9 @@
278281
} do
279282
Dir.stub :glob, ->(pattern) { ['/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor'] } do
280283
capture_io do
281-
cpu_config.configure_for_benchmarking(turbo: false)
284+
cpu_config.stub :keep_sudo_alive, -> {} do
285+
cpu_config.configure_for_benchmarking(turbo: false)
286+
end
282287
end
283288
assert_operator call_count, :>, 0, "Should call check_call to configure AMD CPU"
284289
assert at_exit_called, "Should register at_exit handler to restore CPU settings"
@@ -291,15 +296,13 @@
291296
end
292297
end
293298

294-
# Verify at_exit block restores AMD boost settings
295-
cleanup_commands = []
296-
BenchmarkRunner.stub :check_call, ->(cmd, **opts) { cleanup_commands << { cmd: cmd, opts: opts } } do
299+
# Verify at_exit block restores AMD boost settings non-interactively.
300+
restore_args = nil
301+
cpu_config.stub :system, ->(*args, **_opts) { restore_args = args; true } do
297302
at_exit_block.call
298303
end
299304

300-
assert_equal 1, cleanup_commands.length, "at_exit block should call check_call once"
301-
assert_equal "sudo -S sh -c 'echo 1 > /sys/devices/system/cpu/cpufreq/boost'", cleanup_commands[0][:cmd]
302-
assert_equal({ quiet: true }, cleanup_commands[0][:opts])
305+
assert_equal ["sudo", "-n", "sh", "-c", "echo 1 > /sys/devices/system/cpu/cpufreq/boost"], restore_args
303306
end
304307

305308
it 'exits when AMD boost is not disabled and turbo flag is false' do
@@ -363,3 +366,59 @@
363366
end
364367
end
365368
end
369+
370+
describe 'sudo credential handling' do
371+
# The helpers under test live on the base CPUConfig; any concrete subclass
372+
# exercises them. These guard against the silent at_exit hang where a lapsed
373+
# sudo timestamp left the turbo-restore command blocking on an invisible
374+
# password prompt (stderr was suppressed).
375+
def config
376+
IntelCPUConfig.new
377+
end
378+
379+
describe '#sudo_restore' do
380+
it 'restores non-interactively with sudo -n and stays silent on success' do
381+
cfg = config
382+
captured = nil
383+
fake_system = ->(*args, **_opts) { captured = args; true }
384+
385+
_out, err = capture_io do
386+
cfg.stub :system, fake_system do
387+
cfg.send(:sudo_restore, "sudo sh -c 'echo 0 > /no_turbo'", "echo 0 > /no_turbo")
388+
end
389+
end
390+
391+
assert_equal ["sudo", "-n", "sh", "-c", "echo 0 > /no_turbo"], captured
392+
assert_empty err
393+
end
394+
395+
it 'prints the manual command instead of hanging when credentials lapsed' do
396+
cfg = config
397+
398+
_out, err = capture_io do
399+
cfg.stub :system, false do
400+
cfg.send(:sudo_restore, "sudo sh -c 'echo 0 > /no_turbo'", "echo 0 > /no_turbo")
401+
end
402+
end
403+
404+
assert_match(/credentials expired/, err)
405+
assert_match(%r{sudo sh -c 'echo 0 > /no_turbo'}, err)
406+
end
407+
end
408+
409+
describe '#keep_sudo_alive' do
410+
it 'starts at most one keepalive thread and memoizes it' do
411+
cfg = config
412+
started = 0
413+
fake_thread = Object.new
414+
415+
Thread.stub :new, ->(*_args, &_blk) { started += 1; fake_thread } do
416+
cfg.send(:keep_sudo_alive)
417+
cfg.send(:keep_sudo_alive)
418+
end
419+
420+
assert_equal 1, started
421+
assert_same fake_thread, cfg.instance_variable_get(:@sudo_keepalive)
422+
end
423+
end
424+
end

0 commit comments

Comments
 (0)