Skip to content

Commit 10b7529

Browse files
author
Oana Tanasoiu
committed
(maint) Avoid deadlock of Facter::Core::Execution.execute
1 parent 2bcf4a3 commit 10b7529

2 files changed

Lines changed: 43 additions & 5 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
test_name "Facter::Core::Execution doesn't kill process with long stderr message" do
2+
tag 'risk:high'
3+
4+
confine :except, :platform => /windows/
5+
6+
long_output = "This is a very long error message. " * 4096
7+
file_content = <<-EOM
8+
#!/bin/sh
9+
echo 'newfact=value_of_fact'
10+
1>&2 echo #{long_output}
11+
exit 1
12+
EOM
13+
14+
15+
agents.each do |agent|
16+
17+
external_dir = agent.tmpdir('external_dir')
18+
fact_file = File.join(external_dir, 'test.sh')
19+
create_remote_file(agent, fact_file, file_content)
20+
agent.chmod('+x', fact_file)
21+
22+
teardown do
23+
agent.rm_rf(external_dir)
24+
end
25+
26+
step "Facter: should resolve the external fact and print as warning script's stderr message" do
27+
on agent, facter('--external-dir', external_dir, 'newfact') do |facter_output|
28+
assert_match(/value_of_fact/, facter_output.stdout.chomp)
29+
assert_match(/WARN test.sh .*test.sh resulted with the following stderr message: This is a very long error message./, facter_output.stderr.chomp)
30+
end
31+
end
32+
end
33+
end
34+

lib/facter/custom_facts/core/execution/base.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,18 +100,22 @@ def execute_command(command, on_fail, logger = nil, time_limit = nil)
100100
@log.debug("Executing command: #{command}")
101101
out, stderr = Open3.popen3(opts, command.to_s) do |_, stdout, stderr, wait_thr|
102102
pid = wait_thr.pid
103-
output = +''
104-
err = +''
103+
stdout_messages = +''
104+
stderr_messages = +''
105+
out_reader = Thread.new { stdout.read }
106+
err_reader = Thread.new { stderr.read }
105107
begin
106108
Timeout.timeout(time_limit) do
107-
output << stdout.read
108-
err << stderr.read
109+
stdout_messages << out_reader.value
110+
stderr_messages << err_reader.value
109111
end
110112
rescue Timeout::Error
111113
@log.debug("Timeout encounter after #{time_limit}s, killing process with pid: #{pid}")
112114
Process.kill('KILL', pid)
115+
out_reader.kill
116+
err_reader.kill
113117
end
114-
[output, err]
118+
[stdout_messages, stderr_messages]
115119
end
116120
log_stderr(stderr, command, logger)
117121
rescue StandardError => e

0 commit comments

Comments
 (0)