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

runtime: block console ctrlhandler when the signal is handled #41886

Closed
wants to merge 16 commits into from
Closed

runtime: block console ctrlhandler when the signal is handled #41886

wants to merge 16 commits into from

Conversation

ncruces
Copy link
Contributor

@ncruces ncruces commented Oct 9, 2020

Fixes #41884

I can confirm this change fixes my issue.
I can't confirm that this doesn't break any and everything else.
I see that this code has been tweaked repeatedly, so I would really welcome guidance into further testing.

@google-cla google-cla bot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Oct 9, 2020
@ncruces ncruces changed the title runtime: block the console ctrlhandler when the signal is handled runtime: block console ctrlhandler when the signal is handled Oct 9, 2020
@gopherbot
Copy link
Contributor

This PR (HEAD: 927af81) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Ian Lance Taylor:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 4fcde2f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Gobot Gobot:

Patch Set 2:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 2:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: b6fa877) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 03abc2d) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 3:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 2:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 4:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 54ffa75) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 5:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 4877073) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 6: Run-TryBot+1 Trust+1

(12 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=3c2926a7


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 6: TryBot-Result+1

SlowBots are happy.

SlowBot builds that ran:

  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 084270e) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 9: TryBot-Result+1

SlowBots are happy.

SlowBot builds that ran:

  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 43b589f) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 10:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 10: Run-TryBot+1 Code-Review+2 Trust+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 10:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=b7e53656


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 10:

Build is still in progress...
This change failed on windows-amd64-longtest:
See https://storage.googleapis.com/go-build-log/b7e53656/windows-amd64-longtest_ecd2b0ed.log

Other builds still in progress; subsequent failure notices suppressed until final report. Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 10: TryBot-Result-1

1 of 22 SlowBots failed:
Failed on windows-amd64-longtest: https://storage.googleapis.com/go-build-log/b7e53656/windows-amd64-longtest_ecd2b0ed.log

Consult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test exactly your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.

SlowBot builds that ran:

  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 10:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: decd7b7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 11:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 11: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 11:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=399daaa4


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 11: TryBot-Result+1

SlowBots are happy.

SlowBot builds that ran:

  • windows-amd64-longtest

Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Alex Brainman:

Patch Set 11:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 11:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 11:

(2 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 4ddb2d7) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

This PR (HEAD: 92f02c9) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/261057 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 12: Run-TryBot+1

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 12:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=f0d00f19


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Nuno Cruces:

Patch Set 12:

(3 comments)


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Jason A. Donenfeld:

Patch Set 13: Run-TryBot+1


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Go Bot:

Patch Set 13:

SlowBots beginning. Status page: https://farmer.golang.org/try?commit=4dee3cb3


Please don’t reply on this GitHub thread. Visit golang.org/cl/261057.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Jan 27, 2021
Fixes #41884

I can confirm this change fixes my issue.
I can't confirm that this doesn't break any and everything else.
I see that this code has been tweaked repeatedly, so I would really welcome guidance into further testing.

Change-Id: I1986dd0c2f30cfe10257f0d8c658988d6986f7a6
GitHub-Last-Rev: 92f02c9
GitHub-Pull-Request: #41886
Reviewed-on: https://go-review.googlesource.com/c/go/+/261057
Run-TryBot: Jason A. Donenfeld <Jason@zx2c4.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Jason A. Donenfeld <Jason@zx2c4.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/261057 has been merged.

@gopherbot gopherbot closed this Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

runtime: handling of CTRL_CLOSE_EVENT seems broken
2 participants