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

Could process::Command implement Send ? #47751

Closed
little-dude opened this issue Jan 25, 2018 · 1 comment · Fixed by #47760
Closed

Could process::Command implement Send ? #47751

little-dude opened this issue Jan 25, 2018 · 1 comment · Fixed by #47760
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@little-dude
Copy link
Contributor

Hi,

Currently, we cannot send a process::Command to another thread because it does not implement Send (at least on Unix). The reason is the struct has a raw pointer field:

pub struct Command {
    // Currently we try hard to ensure that the call to `.exec()` doesn't
    // actually allocate any memory. While many platforms try to ensure that
    // memory allocation works after a fork in a multithreaded process, it's
    // been observed to be buggy and somewhat unreliable, so we do our best to
    // just not do it at all!
    //
    // Along those lines, the `argv` and `envp` raw pointers here are exactly
    // what's gonna get passed to `execvp`. The `argv` array starts with the
    // `program` and ends with a NULL, and the `envp` pointer, if present, is
    // also null-terminated.
    //
    // Right now we don't support removing arguments, so there's no much fancy
    // support there, but we support adding and removing environment variables,
    // so a side table is used to track where in the `envp` array each key is
    // located. Whenever we add a key we update it in place if it's already
    // present, and whenever we remove a key we update the locations of all
    // other keys.
    program: CString,
    args: Vec<CString>,
    argv: Vec<*const c_char>,
    env: CommandEnv<DefaultEnvKey>,

    cwd: Option<CString>,
    uid: Option<uid_t>,
    gid: Option<gid_t>,
    saw_nul: bool,
    closures: Vec<Box<FnMut() -> io::Result<()> + Send + Sync>>,
    stdin: Option<Stdio>,
    stdout: Option<Stdio>,
    stderr: Option<Stdio>,
}

According to @cuviper it's probably possible to implement Send manually though. Is this something you would consider?

@sfackler
Copy link
Member

Yeah, we just need a manual Send impl.

little-dude added a commit to little-dude/rust that referenced this issue Jan 25, 2018
@cuviper cuviper added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jan 27, 2018
kennytm added a commit to kennytm/rust that referenced this issue Jan 29, 2018
implement Send for process::Command on unix

closes rust-lang#47751
kennytm added a commit to kennytm/rust that referenced this issue Jan 30, 2018
implement Send for process::Command on unix

closes rust-lang#47751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants