Skip to content

Console.Unix: fix missing terminal configuration on SIGINT/SIGQUIT/SGCONT. - #64200

Merged
stephentoub merged 1 commit into
dotnet:mainfrom
tmds:term_echo
Feb 7, 2022
Merged

Console.Unix: fix missing terminal configuration on SIGINT/SIGQUIT/SGCONT.#64200
stephentoub merged 1 commit into
dotnet:mainfrom
tmds:term_echo

Conversation

@tmds

@tmdstmds commented Jan 24, 2022

Copy link
Copy Markdown
Member

The introduction of the managed API for signal handling (PosixSignal)
inadvertently caused terminal configuration to no longer be performed
when no managed handlers are registered.

This adds back the unconditional registration for SIGINT/SIGQUIT/SIGCONT.

The missing registrations can cause the terminal to stop echoing when an
application terminates on Ctrl-C.

Regressed in #54136.
Fixes#64166.

cc @stephentoub@adamsitnik@danielbisar

@tmds
Console.Unix: fix missing terminal configuration on SIGINT/SIGQUIT/SI…
0d52499
…GCONT.
The introduction of the managed API for signal handling (PosixSignal)
inadvertently caused terminal configuration to no longer be performed
when no managed handlers are registered.
This adds back the unconditional registration for SIGINT/SIGQUIT/SIGCONT.
The missing registrations can cause the terminal to stop echoing when an
application terminates on Ctrl-C.
@ghostghost added area-System.Net community-contribution Indicates that the PR has been added by a community member labels Jan 24, 2022
@ghost

ghost commented Jan 24, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

The introduction of the managed API for signal handling (PosixSignal)
inadvertently caused terminal configuration to no longer be performed
when no managed handlers are registered.

This adds back the unconditional registration for SIGINT/SIGQUIT/SIGCONT.

The missing registrations can cause the terminal to stop echoing when an
application terminates on Ctrl-C.

Regressed in #54136.
Fixes #64166.

cc @stephentoub@adamsitnik@danielbisar

Author:tmds
Assignees:-
Labels:

area-System.Net

Milestone:-

@ghost

ghost commented Jan 24, 2022

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-console
See info in area-owners.md if you want to be subscribed.

Issue Details

The introduction of the managed API for signal handling (PosixSignal)
inadvertently caused terminal configuration to no longer be performed
when no managed handlers are registered.

This adds back the unconditional registration for SIGINT/SIGQUIT/SIGCONT.

The missing registrations can cause the terminal to stop echoing when an
application terminates on Ctrl-C.

Regressed in #54136.
Fixes #64166.

cc @stephentoub@adamsitnik@danielbisar

Author:tmds
Assignees:-
Labels:

area-System.Console, area-System.Net, community-contribution

Milestone:-

stephentoub
stephentoub reviewed Jan 24, 2022
View reviewed changes

@stephentoubstephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. Is there any reasonable way to add a test for this?

@tmds

tmds commented Jan 25, 2022

Copy link
Copy Markdown
MemberAuthor

Is there any reasonable way to add a test for this?

I thought about it, and it's not easy to write an automated test for this. A good test would have something other than .NET as a parent because .NET changes terminal settings also as child processes are created/reaped.

@tmds

tmds commented Feb 1, 2022

Copy link
Copy Markdown
MemberAuthor

@stephentoub@adamsitnik is this good to be merged?

After it sat on main for a while, can we backport it to 6.0?

@stephentoub
stephentoub merged commit e9a7a3f into dotnet:mainFeb 7, 2022
@stephentoub

stephentoub commented Feb 7, 2022

Copy link
Copy Markdown
Member

Thanks, Tom.

@adamsitnikadamsitnik mentioned this pull request Feb 8, 2022
Closed
@adamsitnik

adamsitnik commented Feb 8, 2022

Copy link
Copy Markdown
Member

@tmds thanks for the fix! I am going to start the backport procedure now, as I expect that it will take us few weeks to get it merged

/backport to release/6.0

@adamsitnik

adamsitnik commented Feb 8, 2022

Copy link
Copy Markdown
Member

/backport to release/6.0

@github-actions

github-actionsBot commented Feb 8, 2022

Copy link
Copy Markdown
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1812920689

@github-actionsgithub-actionsBot mentioned this pull request Feb 8, 2022
Merged
@ghostghost locked as resolved and limited conversation to collaborators Mar 10, 2022
@danmoseley

danmoseley commented Jun 14, 2022

Copy link
Copy Markdown
Contributor

I closed the backport PR as stale. cc @dotnet/area-system-console since @tmds is requesting we backport this. One of them would have to champion it, add the template, etc.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub approved these changes

Assignees

No one assigned

Labels

area-System.Consolecommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Console.ReadLine on Ubuntu in WSL disables echo of characters when cancelled with Ctrl-C

4 participants

@tmds@stephentoub@adamsitnik@danmoseley