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

rustc: Switch dsymutil status => output #16064

Closed
wants to merge 1 commit into from

Conversation

alexcrichton
Copy link
Member

Sometimes dsymutil writes to stdout/stderr which rust isn't reading, which may
cause a deadlock.

Closes #16060

Sometimes dsymutil writes to stdout/stderr which rust isn't reading, which may
cause a deadlock.

Closes rust-lang#16060
@lilyball
Copy link
Contributor

Wouldn't it be cleaner to use something like

match Command::new("dsymutil").arg(out_filename).stdout(io::process::Ignored)
                              .stderr(io::process::Ignored).status() {
    Ok(..) => {}
    ...

This way you're not unnecessarily buffering output just to throw it away.

Heck, it looks like reading the output actually spawns two tasks (one for stdout and one for stderr). That's even more wasted work than merely buffering the output.

@lilyball
Copy link
Contributor

As mentioned in #16060 this could even decide to inherit the stderr fd instead of ignoring it, so any dsymutil errors will be visible to the user.

@alexcrichton
Copy link
Member Author

An allocation or a task spawning or two at that stage of compilation is just a drop in the bucket at that point, there's no need to actually worry about it. This has been the source of many annoying warnings being printed in the past, which is why I believe it's silenced.

@lilyball
Copy link
Contributor

Well, ignoring preserves silence. I guess I won't object if you go ahead with .output(), but without a comment someone else may come along later and wonder why you're requesting output and promptly ignoring it. Explicitly closing stdout/stderr makes it clear what you're doing.

@alexcrichton alexcrichton deleted the issue-16060 branch July 30, 2014 14:06
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.

rustc hangs if dsymutil writes to stdout
4 participants