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

elf: fix circular dependency in elf object #30

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abenkhadra
Copy link
Contributor

object elf (parent) owns the memory of its children, namely, sections
and segments. However, instances of the latter share memory ownership
of the parent elf due to its pimpl implementation using shared_ptr.
This creates a circular dependency that prevents cleaning the memory
acquired by elf objects.

I break this circular dependency through the standard mechanism
of weak_ptr. Basically, the children now own a weak_ptr to the
implementation of their parent elf object. They instantiate
an instance of the parent only when an action involving the parent
is needed.

object elf (parent) owns the memory of its children, namely, sections
and segments. However, instances of the latter share memory ownership
of the parent elf due to its pimpl implementation using shared_ptr.
This creates a circular dependency that prevents cleaning the memory
acquired by elf objects.

I break this circular dependency through the standard mechanism
of weak_ptr. Basically, the children now own a weak_ptr to the
*implementation* of their parent elf object. They instantiate
an instance of the parent only when an action involving the parent
is needed.
@axel-capodaglio
Copy link

This can also be solved by just changing "const elf f" member of segment and section to "const elf& f".
Because the elf keeps the children, they are alive as long as the parent is alive (obviously with this solution copies of segments and section must be in scope with the parent elf object).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants