vapor / core

🌎 Utility package containing tools for byte manipulation, Codable, OS APIs, and debugging.
MIT License
82 stars 50 forks source link

use DispatchIO for Process.execute #177

Open tanner0101 opened 6 years ago

tanner0101 commented 6 years ago

This takes a shot at fixing #176 by using DispatchIO.

vzsg commented 6 years ago

One thing that wasn't mentioned in the other issue but in the Discord discussion that presumed it is that those pipes should probably be closed manually after the child process exited. Otherwise, there's a risk of running out of file descriptors when a Vapor app calls external processes often.

tanner0101 commented 6 years ago

@vzsg yeah I think you're right. I've added code to close the read end of the pipes once the DispatchIO stream is closed.

calebkleveter commented 6 years ago

Is there a way to reliably fail the older version of Process.asyncExecute in a test case, so we can verify that this PR fixes it?

tanner0101 commented 6 years ago

@calebkleveter I can't think of a way to do that reliably. :\ But maybe someone else does.

tanner0101 commented 6 years ago

This fix is being held up by a bug with DispatchIO on Linux. I'm talking w/ Apple to see if there's anyway we can do about it.