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

Remove dependency on named_pipe #83

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

alexcrichton
Copy link
Contributor

I was sporadically receiving a segfault locally when trying to debug issues on
Windows and in tracking this down I discovered blackbeam/named_pipe#3 which
leads to segfaults locally on startup.

This switches the one use case to the relevant functionality in
mio-named-pipes (already pulled in as part of tokio-process) and then
otherwise mirrors the same logic as the Unix version, just waiting for a byte
with a timeout.

@alexcrichton
Copy link
Contributor Author

Note that I took this as an opportunity as well to remove the raw call to CreateProcess now that the associated extension method is now stable.

@luser
Copy link
Contributor

luser commented Mar 21, 2017

This looks great (thanks for the CreateProcess cleanup!) There are just two things that I'd like fixed before you merge this:

  1. That windows_process_extensions is also in use in the process spawning code, so you'll break the nightly Travis build with this right now. Just strip out the feature tests there (and remove the no-op version).
  2. Bump the minimum Rust version in .travis.yml and the README.

I was sporadically receiving a segfault locally when trying to debug issues on
Windows and in tracking this down I discovered blackbeam/named_pipe#3 which
leads to segfaults locally on startup.

This switches the one use case to the relevant functionality in
`mio-named-pipes` (already pulled in as part of `tokio-process`) and then
otherwise mirrors the same logic as the Unix version, just waiting for a byte
with a timeout.
@alexcrichton
Copy link
Contributor Author

Certainly, fixed!

@luser luser merged commit 8968eba into mozilla:master Mar 21, 2017
@alexcrichton alexcrichton deleted the fix-named-pipes branch March 21, 2017 22:12
alnr pushed a commit to alnr/sccache that referenced this pull request Aug 3, 2021
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.

2 participants