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.

Sign up for GitHub

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

Jump to bottom

Resolve memory leaks in ConcurrentMap, EpochManager, LockFreeStack, and LockFreeQueue #25928

Merged
merged 4 commits into from
Sep 24, 2024
Merged

Resolve memory leaks in ConcurrentMap, EpochManager, LockFreeStack, and LockFreeQueue #25928

merged 4 commits into from
Sep 24, 2024

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Sep 11, 2024 •
edited
Loading

Resolves all of the memory leaks that are exposed by our current tests in the ConcurrentMap, EpochManager, LockFreeStack, and LockFreeQueue modules

While working on this, I also found #25926 and this PR adds a future for that.

Testing

  • full paratest with/without comm
  • full paratest with -memleaks
  • full paratest with comm and -memleaks

[Reviewed by @stonea]

@jabraham17 jabraham17 marked this pull request as ready for review September 11, 2024 17:00
@jabraham17 jabraham17 requested a review from stonea September 11, 2024 17:00
@jabraham17
update skipif
431c2da 
Signed-off-by: Jade Abraham <[email protected]>
stonea
stonea approved these changes Sep 11, 2024
View reviewed changes
Copy link
Contributor

@stonea stonea left a comment

Choose a reason for hiding this comment

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

Thanks for taking this on. It's certainly nice to get rid of that ugly memLeakTests/ folder!

modules/packages/ConcurrentMap.chpl Show resolved Hide resolved
@jabraham17
add future
5d4f573 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 merged commit afb02c2 into chapel-lang:main Sep 24, 2024
7 checks passed
@jabraham17 jabraham17 deleted the resolve-memleaks branch September 24, 2024 18:12
@jabraham17 jabraham17 mentioned this pull request Sep 25, 2024
Merged
jabraham17 added a commit that referenced this pull request Sep 25, 2024
@jabraham17
Restore skipif for ConcurrentMap and EpochManager (#25990)
e4c48fa 
Restores skipifs removed in
#25928, only 1 line from the
skipif should have been removed

[Reviewed by @stonea]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@e-kayrakli e-kayrakli e-kayrakli left review comments

@stonea stonea stonea approved these changes

Assignees
No one assigned
Labels
post-release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jabraham17 @stonea @e-kayrakli