Skip to content

WIP: fix: Avoid storing attributes in global preserve list#869

Closed
krlmlr wants to merge 1 commit intomainfrom
f-866-un-preserve
Closed

WIP: fix: Avoid storing attributes in global preserve list#869
krlmlr wants to merge 1 commit intomainfrom
f-866-un-preserve

Conversation

@krlmlr
Copy link
Copy Markdown
Contributor

@krlmlr krlmlr commented Jul 2, 2023

The current implementation seems to call R_PreserveObject() and R_ReleaseObject() way too often. This is R's global "preserve" list of objects that are never freed.

With the new implementation, we only add to the global list directly after initializing the attributes. As soon as we return the constructed object as an SEXP to R, the attributes are removed from the global preserve list.

The only problem I see is if we initialize the attributes and then exit with an error. The attribute vector will remain in the global preserve list forever, a memory leak. In fact, we should not use the global preserve list for that, but maintain or own list that we free unconditionally when returning to R.

This is only possible because we now check all igraph function calls and don't longjmp.

@krlmlr krlmlr marked this pull request as draft July 2, 2023 21:31
@krlmlr
Copy link
Copy Markdown
Contributor Author

krlmlr commented Jul 2, 2023

Closing in favor of #870.

@krlmlr krlmlr closed this Jul 2, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant