Skip to content

Fix memory leak in RM_StreamIteratorStop and moduleFreeKeyIterator#10353

Merged
oranagra merged 3 commits into
redis:unstablefrom
vitahlin:vitahlin
Feb 28, 2022
Merged

Fix memory leak in RM_StreamIteratorStop and moduleFreeKeyIterator#10353
oranagra merged 3 commits into
redis:unstablefrom
vitahlin:vitahlin

Conversation

@vitahlin

@vitahlin vitahlin commented Feb 28, 2022

Copy link
Copy Markdown
Contributor

Forget to call streamIteratorStop after streamIteratorStart

oranagra
oranagra reviewed Feb 28, 2022
View reviewed changes

@oranagra oranagra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isn't that needed in moduleFreeKeyIterator too?
@zuiderkwast FYI.

@vitahlin vitahlin changed the title Fix memory leak in RM_StreamIteratorStop Fix memory leak in RM_StreamIteratorStop and add streamIteratorStop to moduleFreeKeyIterator Feb 28, 2022
@vitahlin vitahlin changed the title Fix memory leak in RM_StreamIteratorStop and add streamIteratorStop to moduleFreeKeyIterator Fix memory leak in RM_StreamIteratorStop and moduleFreeKeyIterator Feb 28, 2022
@zuiderkwast

zuiderkwast commented Feb 28, 2022

Copy link
Copy Markdown
Contributor

Good! I wonder why the address sanitizer didn't catch this. Don't we have any test cases which cover the leaky scenarios?

@sundb

sundb commented Feb 28, 2022

Copy link
Copy Markdown
Collaborator

@zuiderkwast Because rax iterator only allocates memory when the key length exceeds 128, or the depth exceeds 32.

@oranagra oranagra merged commit dff153f into redis:unstable Feb 28, 2022
oranagra pushed a commit that referenced this pull request Apr 27, 2022
@vitahlin @oranagra
Fix memory leak in RM_StreamIteratorStop and moduleFreeKeyIterator (#…
2643c69 
…10353)

* Fix memory leak in RM_StreamIteratorStop
* Fix memory leak in moduleFreeKeyIterator

(cherry picked from commit dff153f)
@Yuuoniy Yuuoniy mentioned this pull request May 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oranagra oranagra oranagra approved these changes
+1 more reviewer
@zuiderkwast zuiderkwast zuiderkwast approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

Redis 6.2 Backport
Status: Done
Redis 7.0
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vitahlin @zuiderkwast @sundb @oranagra