Skip to content

Signal handler: don't fire on destruction#602

Merged
mum4k merged 2 commits intoenvoyproxy:masterfrom
oschaaf:signal-handling-fix
Jan 12, 2021
Merged

Signal handler: don't fire on destruction#602
mum4k merged 2 commits intoenvoyproxy:masterfrom
oschaaf:signal-handling-fix

Conversation

@oschaaf
Copy link
Copy Markdown
Member

@oschaaf oschaaf commented Jan 5, 2021

Fixes a bug observed while working on horizontal scaling support:
SignalHandler would fire on destruction.
Fix & add tests.

Note: pre-emptive, I don't think this is an issue in the current
state of master. But some of the CI errors in #600 seemed familiar
and hopefully merging this in there helps.

(Split out from https://github.com/oschaaf/nighthawk/tree/horizontal-scaling)

/cc @eric846

Signed-off-by: Otto van der Schaaf oschaaf@we-amp.com

Fixes a bug observed while working on horizontal scaling support:
SignalHandler would fire on destruction.
Fix & add tests.

Note: pre-emptive, I don't think this is  an issue in the current
state of master. But some of the CI errors in envoyproxy#600 seemed familiar
and hopefully merging this in there helps.

(Split out from https://github.com/oschaaf/nighthawk/tree/horizontal-scaling)

/cc @eric846

Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf oschaaf added the waiting-for-review A PR waiting for a review. label Jan 5, 2021
@oschaaf oschaaf requested a review from eric846 January 5, 2021 23:28
@mum4k mum4k requested a review from qqustc January 6, 2021 00:50
@mum4k
Copy link
Copy Markdown
Collaborator

mum4k commented Jan 6, 2021

@qqustc please review and assign back to me once done.

eric846
eric846 previously approved these changes Jan 8, 2021
Copy link
Copy Markdown
Contributor

@eric846 eric846 left a comment

Choose a reason for hiding this comment

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

LGTM

qqustc
qqustc previously approved these changes Jan 8, 2021
mum4k
mum4k previously approved these changes Jan 8, 2021
Copy link
Copy Markdown
Collaborator

@mum4k mum4k left a comment

Choose a reason for hiding this comment

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

Looks good, please address outstanding comments before we merge this in.

@mum4k mum4k added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Jan 8, 2021
Signed-off-by: Otto van der Schaaf <oschaaf@we-amp.com>
@oschaaf oschaaf dismissed stale reviews from mum4k, qqustc, and eric846 via 455e10a January 8, 2021 23:30
@oschaaf oschaaf added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Jan 9, 2021
@mum4k mum4k merged commit 6b04186 into envoyproxy:master Jan 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-review A PR waiting for a review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants