CA-208547: Avoid races in pointing stdin/stdout/stderr at /dev/null #120
Conversation
This file contains hidden or 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 two calls to Unix.dup are intended to point stdout and stderr at /dev/null - this will normally be the case, as Unix.dup will open the next available file descriptor and stdout/stderr have just been closed. However, there are many ways this can go wrong - if an open() happens in another thread it could mean that... * nullfd <> Unix.stdin - this is checked for, but there is no error handling if this fails * one or both of the Unix.dup calls do not point stdout/stderr at /dev/null as intended, but instead open new file descriptors. This change instead uses Unix.dup2 which atomically closes the second file descriptor and copies it from the first. This also allows the explicit calls to Unix.close to be removed. Finally, nullfd is closed as it is not needed after the calls to Unix.dup2. Signed-off-by: John Else <john.else@citrix.com>
Signed-off-by: John Else <john.else@citrix.com>
|
The unix bits of this look correct now |
Member
|
The patches look fine, but why are the tests failing? |
Collaborator
Author
|
Looks like missing camlp4 packages: |
Signed-off-by: John Else <john.else@citrix.com>
johnelse
force-pushed
the
daemonize-fix-race
branch
from
28382d2 to
9f10c2d
Compare
Collaborator
Author
|
Travis is fixed, jenkins is failing because bigdisk is out of space... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The two calls to Unix.dup are intended to point stdout and stderr at
/dev/null - this will normally be the case, as Unix.dup will open the
next available file descriptor and stdout/stderr have just been closed.
However, there are many ways this can go wrong - if an open() happens
in another thread it could mean that...
handling if this fails
/dev/null as intended, but instead open new file descriptors.
This change instead uses Unix.dup2 which atomically closes the second
file descriptor and copies it from the first. This also allows the
explicit calls to Unix.close to be removed. Finally, nullfd is closed as
it is not needed after the calls to Unix.dup2.