fix segfault in ~Region #3108
Merged
fix segfault in ~Region #3108
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
When debuging deepmodeling#3103, I notice a segfault in `~Region`, preventing the actual error message thrown. `_norm_copy_coord_gpu` replaces `boxt` and `rec_boxt` of `Region` with GPU pointers, runs `deepmd::copy_coord_gpu`, and finnally recover the original pointer that can be deleted. However, `deepmd::copy_coord_gpu` might throw CUDA errors for any reason, and the pointers are not recovered. `~Region` tries to delete a pointer that it doesn't own, causing the segfault. The CUDA error message is not visible due to segfault. The segfault in deepmodeling#2895 may be also caused by it. This PR adds a new constructor to `Region` to accept the external pointers. `~Region` will delete `boxt` and`rec_boxt` only when the pointer is not external. We still need to figure out the reason for the error of `_norm_copy_coord_gpu` behind the segfault. Signed-off-by: Jinzhe Zeng <[email protected]>
github-actions
bot
added
Core
OP
and removed
Test CUDA
Signed-off-by: Jinzhe Zeng <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## devel #3108 +/- ##
==========================================
- Coverage 75.86% 75.85% -0.02%
==========================================
Files 247 247
Lines 25075 25081 +6
Branches 1597 1598 +1
==========================================
+ Hits 19023 19024 +1
- Misses 5114 5118 +4
- Partials 938 939 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Jinzhe Zeng <[email protected]>
wanghan-iapcm
approved these changes
Jan 5, 2024
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.
When debuging #3103, I notice a segfault in
~Region, preventing the actual error message thrown._norm_copy_coord_gpureplacesboxtandrec_boxtofRegionwith GPU pointers, runsdeepmd::copy_coord_gpu, and finnally recover the original pointer that can be deleted. However,deepmd::copy_coord_gpumight throw CUDA errors for any reason, and the pointers are not recovered.~Regiontries to delete a pointer that it doesn't own, causing the segfault. The CUDA error message is not visible due to segfault. The segfault in #2895 may be also caused by it.This PR adds a new constructor to
Regionto accept the external pointers.~Regionwill deleteboxtandrec_boxtonly when the pointer is not external.We still need to figure out the reason for the error of
copy_coord_gpubehind the segfault.