Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

use DispatchIO for Process.execute#177

Open
tanner0101 wants to merge 7 commits intomainfrom
fileio-execute
Open

use DispatchIO for Process.execute#177
tanner0101 wants to merge 7 commits intomainfrom
fileio-execute

Conversation

@tanner0101
Copy link
Member

@tanner0101 tanner0101 commented Oct 17, 2018

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

@tanner0101 tanner0101 added the bug label Oct 17, 2018
@tanner0101 tanner0101 added this to the 3.5.0 milestone Oct 17, 2018
@tanner0101 tanner0101 self-assigned this Oct 17, 2018
@tanner0101 tanner0101 changed the title use NonBlockingFileIO for Process.execute use DispatchIO for Process.execute Oct 17, 2018
@vzsg
Copy link
Member

vzsg commented Oct 17, 2018

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
Copy link
Member Author

@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
Copy link
Member

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
Copy link
Member Author

tanner0101 commented Oct 17, 2018

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

@tanner0101
Copy link
Member Author

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.

Base automatically changed from master to main March 12, 2021 02:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants