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.

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

Fix memory leak of ctx_pool.contexts #471

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

neverpanic
Copy link
Collaborator

@neverpanic neverpanic commented Nov 22, 2024

Description

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2327985

Checklist

  • Code modified for feature bug
  • Test suite updated with functionality tests
  • Test suite updated with negative tests
  • Documentation updated

Reviewer's checklist:

  • Any issues marked for closing are addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible commit messages
  • Coverity Scan has run if needed (code PR) and no new defects were found

@Jakuje
Copy link
Contributor

Jakuje commented Nov 22, 2024

Wondering why we did not catch it with the address sanitizer CI.

@beldmit
Copy link
Collaborator

beldmit commented Nov 22, 2024

@Jakuje I think it's msan, not asan

@Jakuje
Copy link
Contributor

Jakuje commented Nov 22, 2024

@Jakuje I think it's msan, not asan

I think the asan has lsan module which should detect memory leaks too:

https://clang.llvm.org/docs/LeakSanitizer.html

But it looks like it needs ASAN_OPTIONS=detect_leaks=1, which is not present here. Let me add it in separate PR.

The msan on the other hand requires rebuilds of the whole module including dependencies to avoid false positives if I remember well.

@neverpanic
Copy link
Collaborator Author

Are the CI failures expected or a side effect of the free?

@simo5
Copy link
Member

simo5 commented Nov 22, 2024

I broke kryoptic CI, I know how to fix it, we do not need to block on it.

@simo5 simo5 added the covscan Triggers Coverity Scanner label Nov 22, 2024
@github-actions github-actions bot removed the covscan Triggers Coverity Scanner label Nov 22, 2024
@simo5 simo5 added the covscan-ok Coverity scan passed label Nov 22, 2024
@simo5 simo5 merged commit ffb4822 into latchset:main Nov 22, 2024
40 of 42 checks passed
@neverpanic neverpanic deleted the cal-fix-rhbz2327985 branch November 22, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
covscan-ok Coverity scan passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants