Fixes for Memory leak on Quit , Fix support for Stream commands, Fix Crash #28
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
dinesh-murugiah
requested review from
Sasidharan-Gopal,
pratheep-kumar and
samof76
dinesh-murugiah
changed the title
| @@ -561,7 +592,8 @@ void mgmtNoKeyRequest::onallChildRespAgrregate(Common::Redis::RespValuePtr&& val | |||
| if (!pending_responses_.empty()) { | |||
| Common::Redis::RespValuePtr response = std::move(pending_responses_[response_index_]); | |||
| callbacks_.onResponse(std::move(response)); | |||
| pending_responses_.clear(); | |||
| //pending_responses_.clear(); | |||
There was a problem hiding this comment.
since you're commenting out the code, can you add a comment explaining why
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.
Commit Message:
Fixes for Memory leak on Quit , Fix support for Stream commands, Fix Crash
Additional Description:
This PR has fixes for the following
Memory leak
There was a memory leak due to proxy filter object not freed , as the downstream callbacks was not cleared . this happened in only when quit command is called . that too quit command without remote close
A wrapper function was added to free up the downstream objects before closing down stream connections locally
Stream command support
Fixes for properly supporting all stream commands catergory is added. stream blocking and non blocking comands are handled seperately
Possible fixes for Random
The reason for random crash is identified as trying to access freed memeory
this is based on the understanding of the last point in which the logs have stopped and result from running address sanitiser , anyhow recomendation would be to still recreate the problem and get a back trace
fixes have been done to not access already freed memory
Risk Level:
Testing:
Basic sanity testing done
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]