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

Correctly destruct AddressSpace objects #871

Merged
merged 2 commits into from
Oct 2, 2020
Merged

Correctly destruct AddressSpace objects #871

merged 2 commits into from
Oct 2, 2020

Conversation

hainest
Copy link
Contributor

@hainest hainest commented Sep 29, 2020

deleteAddressSpace is still highly suspect because its only use is copyAddressSpace which doesn't fully copy the object. However, this fix at least makes deleteAddressSpace and ~AddressSpace correct and perform the same actions.

This was originally part of #317.

@hainest hainest added bug code cleanup Bring the code up to modern standards or remove deprecated features labels Sep 29, 2020
@hainest hainest requested a review from mxz297 September 29, 2020 03:31
@hainest hainest self-assigned this Sep 29, 2020
@hainest
Copy link
Contributor Author

hainest commented Sep 30, 2020

https://bottle.cs.wisc.edu/search?dyninst_branch=PR871

No regressions (cori@NERSC is offline right now).

@mxz297
Copy link
Member

mxz297 commented Oct 1, 2020

@hainest Looks good. I don't think this memory leak fix will make much difference as AddressSpace objects do not get destructed. But this is still good to fix.

@hainest
Copy link
Contributor Author

hainest commented Oct 2, 2020

AddressSpace objects do not get destructed.

Yikes. I did some digging and found that there are circular lifetimes between BinaryEdit and mapped_object which prevents correct destruction. I'll add it to my TODO list...

@hainest hainest merged commit d3a57b3 into dyninst:master Oct 2, 2020
@hainest hainest deleted the cleanup_AddressSpace branch October 2, 2020 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug code cleanup Bring the code up to modern standards or remove deprecated features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants