(maint) Avoid deadlock of Facter::Core::Execution.execute

[oanatmaria]

Description of the problem: If stdout/stderr are N bytes, and the child process writes N+1 bytes to stderr, but nothing to stdout, then facter will block waiting to read from stdout, hit the timeout, and never read from stderr.
Description of the fix Facter will create two reader threads to avoid the deadlock

[gimmyxd]

You can make use of ruby here, to generate the needed text, eg:

length = 1000
file_content = <<-EOM
   #!/bin/sh
   long_output=#{"Lorem ipsum dolor sit amet" * length }
   echo 'newfact=value_of_fact'
   1>&2 echo $long_output
   exit 1
EOM

And play with the length to achieve the expected output.

Also i think would be useful to add some description to this scenario and what is this test doing

[gimmyxd]

why not use a simple initialize?

stdout_messages = ''
stderr_messages = ''

[oanatmaria]

they will be treated as frozen strings and this will generate an error later when we try to append the output of the command to these strings

[joshcooper]

I've started using String.new to make it clearer that I'm creating a string that isn't frozen

[oanatmaria]

Done

[oanatmaria]

Unfortunately i get the following error when i try to use String.new : lib/facter/custom_facts/core/execution/base.rb:104:33: C: Performance/UnfreezeString: Use unary plus to get an unfrozen string literal.

[puppetcla]

CLA signed by all contributors.

[oanatmaria]

Jenkins please test this on all

[joshcooper]

If stdout.read (on line 105) raises then ruby will raise that exception when out_reader.value is called. So it's possible for line 109 to raise something like Errno::EIO, in which case we never call err_reader.value. As a result we may never join that thread and fully read stderr. I don't know if that's going to be an actual problem though...

[oanatmaria]

If out_reader.value raises an error, the error message will be logged (if execution is called with on_fail => :raise line 122). If the stdout will raise an error, the resolvers can't get the information needed for facts to be solved, the stderr message can only be used for debug purposes in this case.

[joshcooper]

Can Process.kill raise such as if the child process no longer exists? If so then kill is never called on the readers.

[oanatmaria]

i wrapped the threads kill in an ensure block