Skip to content

Commit

Permalink
(MODULES-8358) Chunk read the pipe from the PowerShell Process
Browse files Browse the repository at this point in the history
Previously the PowerShell Manager would attempt all the expected bytes from
the inbound pipe.  Normally this works fine.  However the Travis CI testing
was intermittently failing where the pipe was only reading half the pipe,
which then caused the first test to fail, and then any subsequent tests failed
because it was reading left-over data from the previous failed read.

The documentation of the IO.sysread method [1] says the argument is maxlen
which implies that this the maximum number of bytes it would return.  Note that
other methods in IO use the language "Reads at most maxlen bytes from ios"
whereas sysread uses the langauge "Reads maxlen bytes from ios"

This commit changes the pipe reader to read in chunks of bytes until all of
the expected bytes are read.  Note that this is a blocking call so if you
try to read beyond the bytes in the pipe it blocks the thread.  The value
of 8K chunk was an arbitrary number which is big enough to get most powershell
repsonse in a single IO read.  The integration tests were failing to read in
a 96K chunk (only ~36K were read)

[1] https://ruby-doc.org/core-2.6.1/IO.html#method-i-sysread
  • Loading branch information
glennsarti committed Feb 15, 2019
1 parent cca8fae commit b8db256
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
14 changes: 11 additions & 3 deletions lib/puppet_x/puppetlabs/powershell/powershell_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +394,18 @@ def read_streams
pipe_reader = Thread.new(@pipe) do |pipe|
# read a Little Endian 32-bit integer for length of response
expected_response_length = pipe.sysread(4).unpack('V').first
return nil if expected_response_length == 0

# reads the expected bytes as a binary string or fails
pipe.sysread(expected_response_length)
if expected_response_length == 0
nil
else
# reads the expected bytes as a binary string or fails
buffer = ""
# Reads in the pipe data 8K, or less, at a time
while (buffer.length < expected_response_length)
buffer << pipe.sysread([expected_response_length - buffer.length, 8192].min)
end
buffer
end
end

Puppet.debug "Waited #{Time.now - start_time} total seconds."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,8 @@ def output_cmdlet(ps_command, ps_args)

it "should be able to write more than the 64k default buffer size to the managers pipe without deadlocking the Ruby parent process or breaking the pipe" do
# this was tested successfully up to 5MB of text
buffer_string_96k = 'a' * ((1024 * 96) + 1)
# we add some additional bytes so it's not always on a 1KB boundary and forces pipe reading in different lengths, not always 1K chunks
buffer_string_96k = 'a' * ((1024 * 96) + 11)
result = manager.execute(<<-CODE
'#{buffer_string_96k}' | #{output_cmdlet(ps_command, ps_args)}
CODE
Expand All @@ -572,15 +573,16 @@ def output_cmdlet(ps_command, ps_args)
end

it "should be able to write more than the 64k default buffer size to child process stdout without deadlocking the Ruby parent process" do
# we add some additional bytes so it's not always on a 1KB boundary and forces pipe reading in different lengths, not always 1K chunks
result = manager.execute(<<-CODE
$bytes_in_k = (1024 * 64) + 1
$bytes_in_k = (1024 * 64) + 11
[Text.Encoding]::UTF8.GetString((New-Object Byte[] ($bytes_in_k))) | #{output_cmdlet(ps_command, ps_args)}
CODE
)

expect(result[:errormessage]).to eq(nil)
expect(result[:exitcode]).to eq(0)
expected = "\x0" * (1024 * 64 + 1) + line_end
expected = "\x0" * (1024 * 64 + 11) + line_end
expect(result[:stdout].length).to eq(expected.length)
expect(result[:stdout]).to eq(expected)
end
Expand Down

0 comments on commit b8db256

Please sign in to comment.