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.

Sign up for GitHub

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

Jump to bottom

fix: Remove Stop() function from stress relief #645

Merged
merged 2 commits into from
Apr 2, 2023
Merged

fix: Remove Stop() function from stress relief #645

merged 2 commits into from
Apr 2, 2023

Conversation

kentquirk
Copy link
Contributor

@kentquirk kentquirk commented Mar 28, 2023 •
edited
Loading

Which problem is this PR solving?

  • A customer saw refinery crashes due to trying to close the StressRelief.Done channel twice.

Short description of the changes

  • Remove Stop() function, which was implemented to let the injection system shut down Stress Relief (symmetric to Start()). We don't need it, because the only thing it did was close the Done channel. The Done channel is closed externally by main() during shutdown and you can't close a channel more than once. This will prevent a confusing crash on shutdown.

@kentquirk
Remove Stop() function from stress relief
e72d678 
We don't need it, because the Done channel is closed externally and you can't close a channel more than once.

A customer saw refinery crashes due to this.
@kentquirk kentquirk requested a review from a team as a code owner March 28, 2023 18:31
@kentquirk kentquirk added this to the v1.21 milestone Mar 28, 2023
@kentquirk kentquirk merged commit 6503c90 into main Apr 2, 2023
@kentquirk kentquirk deleted the kent.fix_stress_crash branch April 2, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TylerHelmuth TylerHelmuth TylerHelmuth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.21
Development

Successfully merging this pull request may close these issues.
2 participants
@kentquirk @TylerHelmuth