Improve platform support (chroot environments, Mac and others) and do not inherit open FDs to SSH child process by overwriting and closing #10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The virtual /proc/self/fd is Linux-only, while the virtual /dev/fd is available on more platforms. On Linux, the latter is simply a symlink to the former, so this shouldn't affect existing installations.
If we can not read from /dev/fd (which is available on Linux, Mac and many others), we otherwise try temporarily duplicating file descriptors in the range 0-1024 (FD_SETSIZE) to see which one is currently in use. This is known to work on more exotic platforms and also inside chroot environments without /dev/fd. Causes many syscalls, but still rather fast.
This fixes a possible race condition where open FDs where in fact inherited to the wrapping shell before it had a chance to close them again when it is being replaced with the actual SSH binary. This builds on top of reactphp/child-process#65
clue
added a commit
to clue-labs/reactphp-sqlite
that referenced
this pull request
Jan 22, 2019
This somewhat obscure PR ensures that we do not inherit open file descriptors (FDs) to the SQLite child worker process. This can cause all sorts of errors in long running applications and really is not desired here. This is implemented by explicitly overwriting all superfluous FDs with dummy file handles and then closing all of these in the implicit `sh` child process before launching the actual php binary. PHP does not support `FD_CLOEXEC`, `O_CLOEXEC` or `SOCK_CLOEXEC` and this appears to be the best work around I could find (yes, I should probably write a lengthy, somewhat technical blog post about this). Additionally, this PR includes a test to verify this works on all supported platforms and this could perhaps be used as a starting point for other libraries (YMMV). This builds on top of clue/reactphp-ssh-proxy#2 and clue/reactphp-ssh-proxy#10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a possible race condition where open FDs where in fact
inherited to the wrapping shell before it had a chance to close them
again when it is being replaced with the actual SSH binary. This builds
on top of reactphp/child-process#65
This also improves platform support by checking /dev/fd instead of /proc/self/fd.
The virtual /proc/self/fd is Linux-only, while the virtual /dev/fd is
available on more platforms. On Linux, the latter is simply a symlink to
the former, so this shouldn't affect existing installations.
Additionally, this improves platform support by checking all available FDs as fallback.
If we can not read from /dev/fd (which is available on Linux, Mac and
many others), we otherwise try temporarily duplicating file descriptors
in the range 0-1024 (FD_SETSIZE) to see which one is currently in use.
This is known to work on more exotic platforms and also inside
chroot environments without /dev/fd. Causes many syscalls, but still
rather fast.
Builds on top of #2
Refs reactphp/child-process#51