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

Fix a memory leak in Open BC solver #4290

Merged
merged 1 commit into from
Jan 9, 2025
Merged

Fix a memory leak in Open BC solver #4290

merged 1 commit into from
Jan 9, 2025

Conversation

WeiqunZhang
Copy link
Member

We forgot a test to make sure We only build the plans once.

X-Ref:

@WeiqunZhang
Fix a memory leak in Open BC solver
5eb85b5 
We forgot a test to make sure We only build the plans once.
@WeiqunZhang WeiqunZhang mentioned this pull request Jan 9, 2025
Closed
@WeiqunZhang WeiqunZhang added the bug label Jan 9, 2025
This was referenced Jan 9, 2025
Merged
Closed
@WeiqunZhang WeiqunZhang requested a review from ax3l January 9, 2025 19:51
@ax3l
Copy link
Member

ax3l commented Jan 9, 2025 •
edited
Loading

I saw there is also a defined2 variable for another plan, does related code need to be updated/guarded as well?

ax3l
ax3l approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Tested, fixes the leaks we saw in ImpactX MPI runs 🎉

@ax3l ax3l self-assigned this Jan 9, 2025
@ax3l ax3l merged commit 041f225 into AMReX-Codes:development Jan 9, 2025
75 checks passed
@ax3l ax3l mentioned this pull request Jan 9, 2025
Merged
1 task
@WeiqunZhang WeiqunZhang deleted the openbc_memleak branch January 9, 2025 20:54
@WeiqunZhang
Copy link
Member Author

I saw there is also a defined2 variable for another plan, does related code need to be updated/guarded as well?

No. The bug is limited to the open bc solver. The other plan is used for other types of solvers with mixed boundaries.

@ax3l
Copy link
Member

ax3l commented Jan 9, 2025

Ok perfect. Thank you again! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ax3l ax3l ax3l approved these changes

Assignees

@ax3l ax3l

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WeiqunZhang @ax3l