Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Use pipe2 and O_CLOEXEC to check if exec succeeds after fork #8126

Closed
wants to merge 1 commit into from

Conversation

smvv
Copy link
Contributor

@smvv smvv commented Jul 30, 2013

The problem with issue #8002 and #6914 is that the exec() call fails after fork(). After the failure, the parent process does not know about the failure. The parent process hangs during the waitpid() call.

I tried to use pipe2 and O_CLOEXEC to check in the parent process if exec() succeeds after fork().

This PR fixes the problem with `pandoc'. I tested this on the latest master revision.

$ rustdoc --output-dir doc btree.rs
rust: task failed at 'failure in execvp: errno=No such file or directory', /home/smvv/work/rust/rust/src/libstd/run.rs:689

However, there is still some work to do before it can be merged:

  • Code has a hardcoded O_CLOEXEC value. This should be derived from the C header file. What would be the approach to derive this properly? O_CLOEXEC is defined in: http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
    It is also possible to use fcntl() to set FD_CLOEXEC, but the fcntl defined in libc.rs takes only two arguments instead of three.
  • Code uses pipe2, which is not defined in libc.rs. Should pipe2 be added to libc.rs, or what would be the way to proceed here?
  • There are some comments in the patch about false being used in the FILE reader and writer constructor. I was not sure if it should use false or true.

I read that pipe2 is Linux-specific, but perhaps it is also available on other Unix platforms?

@brson
Copy link
Contributor

brson commented Jul 31, 2013

@graydon is probably the best to answer this, but I can take some guesses. Since pipe2 is linux-specific it should probably be in libc::funcs::extra, and O_CLOEXEC in libc::consts::extra. The code using them would need to be configured only for linux. All the C constants are currently just hardcoded for all platforms, so that's fine.

Since this is a mac problem to, as indicated by the linked issues, I wonder if there are other ways to solve this.

The bool to FILE_reader should be true if you want the reader to close the fd when destroyed.

@thestinger
Copy link
Contributor

I think we should be using posix_spawn here. It will mean we have to set CLOEXEC instead of closing files after a fork, but we should be doing that anyway (node.js does this, so libuv might already be getting us most of the way there). I'm going to close this because we should have a cross-platform solution.

@thestinger thestinger closed this Aug 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants