signal-upgrade close of closed channel #1190
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1190 +/- ##
==========================================
- Coverage 80.84% 79.75% -1.10%
==========================================
Files 149 153 +4
Lines 17042 17402 +360
==========================================
+ Hits 13778 13879 +101
- Misses 2597 2852 +255
- Partials 667 671 +4 ☔ View full report in Codecov by Sentry. |
xxx7xxxx
reviewed
Jan 8, 2024
| @@ -136,5 +139,7 @@ func (m *dynamicMux) reloadAPIs() { | |||
| } | |||
|
|
|||
| func (m *dynamicMux) close() { | |||
| close(m.done) | |||
| m.closeOnce.Do(func() { | |||
There was a problem hiding this comment.
It's rare to support reclosable in Easegress, so it's better to add a comment here.
There was a problem hiding this comment.
updated! please check
xxx7xxxx
approved these changes
Jan 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in issue #1170 , when use
signal-upgradeto graceful update easegress, it will cause a panic with message "close of closed channel". Although it not influence the usage of easegress, but we'd better to fix it.After some carefully digging, here is what happens. When use
signal-upgrade,easegress-serverreceive a user signal, then graceful upgrade. What happens here iseasegress-servergracefully closes api-server and starts a new process (like fork, start a new process with original environment and configuration). And when the new process is started, it will send a signal to old process to terminate, which will close the api-server again, cause the panic with message "close of closed channel".Because both two close of api-server happens in original process, it won't influence the new process.
But since this may influence the core part of easegress. Any fix with a lot of code change may cause potential bugs. So, in this pr, I fix it by making api-server recloseable.