-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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] prevent memory leak if pointer fails to allocate #4613
Conversation
/gha run r-valgrind Workflow R valgrind tests has been triggered! 🚀 Status: failure ❌. |
/gha run r-solaris Workflow Solaris CRAN check has been triggered! 🚀 solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.2.1.99.tar.gz-3d775c70bfe84fdca5ca15b055686492 |
The failing checks don't show any errors. |
On https://github.com/microsoft/LightGBM/runs/3669905286?check_suite_focus=true, I see
And that the job runtime was 3 hours, which is the maximum we allow. LightGBM/.github/workflows/r_valgrind.yml Line 10 in c3b60c2
This happens sometimes and is usually something transient related to the infrastructure behind GitHub Actions. I'll re-run the valgrind test. |
/gha run r-valgrind Workflow R valgrind tests has been triggered! 🚀 Status: success ✔️. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like all the tests are passing, and I understand what's going on in this change. Thanks for the help!
This pull request 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. |
As a follow up from #4597, this PR adds preventions in case an R external object pointer object fails to allocate. This should be an incredibly uncommon situation (failing to allocate 8 bytes), but it is in any case preventable.