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

cargo doc --open broken on Windows due to rustup proxy #1493

Closed
Seeker14491 opened this issue Aug 30, 2018 · 2 comments
Closed

cargo doc --open broken on Windows due to rustup proxy #1493

Seeker14491 opened this issue Aug 30, 2018 · 2 comments

Comments

@Seeker14491
Copy link
Contributor

The cargo doc --open command is currently broken on nightly Cargo on Windows. The implementation of this command recently changed in Cargo. This new implementation is affected by a bug in rustup's job object code on Windows. Job objects are a Windows feature that rustup uses to implement the behavior such that when ctrl-c is pressed, child processes are also killed. The existing job object code was copied from Cargo. I recently fixed the job code in Cargo, which was also causing issues.

I intended on sending a pull request to rustup to apply the same fix, but I ran into a problem: The fix relies on a destructor to run when rustup exits, but rustup uses std::process::exit, which doesn't run destructors. I can think of two solutions; either move the std::process::exit call(s) outwards so that the calls can run after the destructors, or use a mechanism other than a destructor to run the necessary code.

Links
My branch with the fixed job code: https:/Seeker14491/rustup.rs/tree/job
Line in rustup where the object whose destructor needs to run is created: https:/rust-lang-nursery/rustup.rs/blob/22b6cdb8b000f9217af1dd5dd4e73ce032bdb25d/src/rustup-cli/proxy_mode.rs#L14

@alexcrichton
Copy link
Member

Sorry for the delay here @Seeker14491 are you still interested in working on this? Moving the calls to exit outwards seems like the best solution to me! rust-lang/cargo#6029 is indeed a worrying issue and would be great to fix soon!

@Seeker14491
Copy link
Contributor Author

Sure, I'll give it a shot.

Seeker14491 added a commit to Seeker14491/rustup.rs that referenced this issue Sep 19, 2018
The existing code was copied from Cargo. Issues with this code have been found and fixed recently (rust-lang/cargo#5887). This commit updates rustup's copy of the code to match Cargo's.

Fixes rust-lang#1493
Seeker14491 added a commit to Seeker14491/rustup.rs that referenced this issue Sep 19, 2018
The existing code was copied from Cargo. Issues with this code have been found and fixed recently (rust-lang/cargo#5887). This commit updates rustup's copy of the code to match Cargo's.

Fixes rust-lang#1493
Seeker14491 added a commit to Seeker14491/rustup.rs that referenced this issue Sep 19, 2018
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

No branches or pull requests

2 participants