Subprocess 'leaks' open FDs until the Process objects are garbage-collected

[rbroemeling]

While using the subprocess module to execute some fairly simple commands, I noticed that my script was using far more FDs than it should have been (50+, when it should have been <= 10). It took me quite a while to track down -- the problem is that Subprocess::Process#communicate opens an IO.pipe that is never closed (until the GC closes it when the Process object is collected).

I was able to fix the issue by adding:

self_read.close
self_write.close

... on lines 426 and 427 of subprocess.rb, directly after the end of the self.class.catching_sigchld(pid, self_write) block (on GitHub current master, this is line 429: https://github.com/stripe/subprocess/blob/master/lib/subprocess.rb#L429 ).

AFAICT this shouldn't have any effect on the overall operation of the class, but it does help keep the number of the FDs that the script has open down and more representative of what is actually in-use.

Could we get this change rolled into the subprocess module, please?

[rbroemeling]

Just for full disclosure, here's an example of use (and the symptom) with and without my suggest change:

Without the change, showing the FD leak:

[1] pry(main)> require 'subprocess'
=> true
[2] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 11
=> nil
[3] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 13
=> nil
[4] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 15
=> nil
[5] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 17
=> nil
[6] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 19
=> nil
[7] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 21
=> nil

With the change (and thus with no FD leak):

[1] pry(main)> require 'subprocess'
=> true
[2] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil
[3] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil
[4] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil
[5] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil
[6] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil
[7] pry(main)> Subprocess.call(['/usr/bin/uptime'], stdout: Subprocess::PIPE) { |child| child.communicate }; puts "FD count: #{Dir["/proc/#{$$}/fd/*"].length}"
FD count: 9
=> nil

[rbroemeling]

This bug no longer exists in recent versions of the subprocess gem (as reflected in this source code repository, if not yet on rubygems -- see #46). As the bug has been fixed, I'd suggest that this issue can be closed.