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

[R-package] R memory leaks #4282

Closed
david-cortes opened this issue May 12, 2021 · 4 comments · Fixed by #4597
Closed

[R-package] R memory leaks #4282

david-cortes opened this issue May 12, 2021 · 4 comments · Fixed by #4597

Comments

@david-cortes
Copy link
Contributor

There are some situations in which the R version can leak memory by not destructing C++ objects that become unreachable:

  • Functions that return an R external pointer are wrapped into R6 classes, with the finalizer for the external pointer being under the R6 finalizer. If an error occurs during initialization (when calling Class$new(...), like it is done when creating a dataset), the finalizer will not be called, thus leaking the C++ object (e.g. a dataset or a model). This could be solved by setting the finalizer in the R external pointer object itself.
  • Some of the C++ functions allocate R vectors, such as these:
    feature_names = PROTECT(Rf_allocVector(STRSXP, len));

    eval_names = PROTECT(Rf_allocVector(STRSXP, len));

    model_str = PROTECT(Rf_allocVector(STRSXP, 1));

    model_str = PROTECT(Rf_allocVector(STRSXP, 1));

    If the allocation of the R vectors fail, it will trigger a long jump which would leave the C++ vectors that were allocated before undestructed and unreachable. This could be solved by using unwind protection as described in the R extensions manual.
  • Same as per the other issues, calling Rf_error also triggers long jumps. Could either change it to something else while throwing the error on the R side, or add unwind protection for those too.
@jameslamb
Copy link
Collaborator

Thanks for writing this up!

unwind protection as described in the R extensions manual

Are you referring to https://cran.r-project.org/doc/manuals/r-release/R-exts.html#Condition-handling-and-cleanup-code? In the future, we'd very much appreciate if you included relevant links or supporting evidence when opening issues like this or leaving comments like #4273 (comment).

@jameslamb jameslamb changed the title R memory leaks [R-package] R memory leaks May 13, 2021
@david-cortes
Copy link
Contributor Author

Yes, that's what I was referring to.

@jameslamb
Copy link
Collaborator

Ok perfect, thanks very much!

@jameslamb jameslamb mentioned this issue May 20, 2021
21 tasks
@jameslamb jameslamb self-assigned this Jun 8, 2021
StrikerRUS pushed a commit that referenced this issue Sep 17, 2021
* fix R memory leaks

* attempt at solving linter complaints

* fix compilation on windows

* move R_API_BEGIN to correct place

* make sure exception objects reach out of scope

* better way to solve rchk complaints

* remove goto statement
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants