Repository navigation
feat: support to close percentile to prevent from heap-use-after-free error - #1074
Merged
Merged
Conversation
empiredan
marked this pull request as ready for review
July 26, 2022 15:41
empiredan
marked this pull request as draft
July 26, 2022 15:42
acelyc111
reviewed
Jul 27, 2022
acelyc111
approved these changes
Jul 27, 2022
foreverneverer
approved these changes
Jul 28, 2022
GehaFearless
pushed a commit
to GehaFearless/incubator-pegasus
that referenced
this pull request
Aug 3, 2022
Closed
ZhongChaoqiang
pushed a commit
to ZhongChaoqiang/incubator-pegasus
that referenced
this pull request
Nov 28, 2022
…nloading to avoid io-read workload (apache#1074)
This file contains hidden or 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
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.
This PR is to resolve the issue #1066.
The root cause of issue #1066 is that the percentile is freed during the destruction of the metric registry, after which sometimes the handler for asynchronous wait operation of timer will be still executed, where the freed percentile is certainly referenced. This will lead to heap-use-after-free error.
The solution for this problem will be extend the lifetime of percentile: during the construction of the percentile, it can increment its ref count (its base class is
ref_counter) since it is referenced by timer. This effectively extends the lifetime of percentile: even if both of its metric entity and registry are destructed, it can still survive in the memory for the reason that its ref count is 1 at least.Once the percentile should be released when it will retire from metric entity or the metric registry (singleton) will be destructed, the
close()method for percentile will be invoked.close()will change the state of timer from running to closing, and also callcancel()for the timer. Then, whether the timer is cancelled successfully or not, the timer will finally be stopped: if successful, the callback for cancel will be executed immediately; otherwise, the closing state will tell the timer to stop finally, albeit sometimes after a while.