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

wincolor: Re-fetch the console on all calls #523

Merged
merged 1 commit into from
Jun 19, 2017

Conversation

alexcrichton
Copy link
Contributor

The primary motivation for this commit was rust-lang/cargo#4189 where dropping a
wincolor::Console would call CloseHandle to close the console handle. Cargo
creates a few Console instances so it ended up closing stdout a little
earlier as intended!

The GetStdHandle function returns handles I believe aren't intended to be
closed (as there's no refcounting). I believe libstd doesn't close these
handles.

This commit also moves to calling GetStdHandle on demand which libstd changed
to doing so recently as well, preventing caching of stale handles that change
over time with calls to SetStdHandle.

The primary motivation for this commit was rust-lang/cargo#4189 where dropping a
`wincolor::Console` would call `CloseHandle` to close the console handle. Cargo
creates a few `Console` instances so it ended up closing stdout a little
earlier as intended!

The `GetStdHandle` function returns handles I believe aren't intended to be
closed (as there's no refcounting). I believe libstd doesn't close these
handles.

This commit also moves to calling `GetStdHandle` on demand which libstd changed
to doing so recently as well, preventing caching of stale handles that change
over time with calls to `SetStdHandle`.
@BurntSushi BurntSushi merged commit 7763c98 into BurntSushi:master Jun 19, 2017
@BurntSushi
Copy link
Owner

All right, I buy it!

I feel like there was a reason why I wasn't re-creating the console, but my guess is that it was an attempt at a premature optimization.

@alexcrichton alexcrichton deleted the no-close-windows branch June 19, 2017 17:11
@alexcrichton
Copy link
Contributor Author

Thanks! Mind publishing a new version as well?

@BurntSushi
Copy link
Owner

Ah right, 0.1.4 is on crates.io!

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