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

Check enablement state for all output write operations #1581

Merged

Conversation

hackean-msft
Copy link
Contributor

@hackean-msft hackean-msft commented Oct 12, 2021

The color contamination fix addressed by #1527 routes all output write operations through OutputStream including Non virtual terminal operations. The fix introduced a bug causing a write operation without checking the enablement state of the output stream. The fix addressing issue by checking if the write operation should be permitted.

The main output stream that controls all write operation is never cloned when the main context is cloned, this causes the output stream to be disabled upon the destruction of a subcontext(because we disable the output stream to prevent multiple background jobs from writing to the output stream, when user force terminates). This pull request fixes this issue by allowing disabling of BaseOutputStream through forceful termination only.


Microsoft Reviewers: Open in CodeFlow

@hackean-msft hackean-msft requested a review from a team as a code owner October 12, 2021 18:11
Copy link
Contributor

@jedieaston jedieaston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Thanks!

Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@hackean-msft hackean-msft merged commit 5133303 into microsoft:master Oct 14, 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.

Regression: upgrade and import no longer print anything to the console.
4 participants