rust-lang / jobserver-rs

Apache License 2.0
69 stars 40 forks source link

Client::configure is unsafe #25

Open glandium opened 4 years ago

glandium commented 4 years ago

If you do something like:

if let Some(client) = unsafe { jobserver::Client::from_env() } {
    client.configure(&mut cmd);
}

Where cmd is an existing Command.

By the time you execute the command and the pre_cmd that Client::configure set up, the Client instance has already been dropped. And dropping the Client actually closes the file descriptors (which is documented in Client::from_env. So by the time the pre-command executes, the file descriptors are either closed, or worse, were opened for something else.

It seems like Client::configure should have an explicit requirement on the lifetime of the command not exceeding that of the client.

alexcrichton commented 4 years ago

I think we could probably fix this by closing over the necessary state in the closure we pass to Command, it should all be in an internal Arc anyway