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

Fix hangs on SIGTERM. #1480

Closed
wants to merge 1 commit into from
Closed

Fix hangs on SIGTERM. #1480

wants to merge 1 commit into from

Conversation

armooo
Copy link
Contributor

@armooo armooo commented May 27, 2023

This fixes #1461, where two bugs prevent main from existing on SIGTERM.

The first is a blocking receiving from a cancel chanel which never have a, cancelStateUpdateChan. It seems closing the chanel should be all the is needed to signal the watching goroutines to exit.

The other is the signal hander never exiting an infinite for loop. The sigFunc is called in an errorGroup which blocks exiting Serve and thus main. It looks like a refactor in #1382 removed an os.Exit(0), replacing it with a return breaks out of the loop.

  • read the CONTRIBUTING guidelines
  • raised a GitHub issue or discussed it on the projects chat beforehand
  • added unit tests
  • added integration tests
  • updated documentation if needed
  • updated CHANGELOG.md

This fixes juanfont#1469, where three bugs pervent main from existing on
SIGTERM.

The first two are blocking receiving from cancel chanels which never
have values sent to them, cancelStateUpdateChan and
cancelPolicyUpdateChan. It seems closing the chanels should be all the
is needed to signal the watching goroutines to exit.

The other is the signal hander never exiting an infinite for loop. The
sigFunc is called in an errorGroup which blocks exiting Serve and thus
main. It looks like a refactor in juanfont#1382 removed an os.Exit(0),
replacing it with a return breaks out of the loop.
@kradalby
Copy link
Collaborator

kradalby commented Jul 7, 2023

Picking this up in #1492, added you as a co-author.

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.

Strange shutdown/restart behavior on 0.22.2
2 participants