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] prevent memory leak if pointer fails to allocate #4613

Merged
merged 1 commit into from
Sep 22, 2021

Conversation

david-cortes
Copy link
Contributor

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.

@jameslamb
Copy link
Collaborator

jameslamb commented Sep 22, 2021

/gha run r-valgrind

Workflow R valgrind tests has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/1259876731

Status: failure ❌.

@jameslamb
Copy link
Collaborator

jameslamb commented Sep 22, 2021

/gha run r-solaris

Workflow Solaris CRAN check has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/1259876933

solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.2.1.99.tar.gz-3d775c70bfe84fdca5ca15b055686492
solaris-x86-patched-ods: https://builder.r-hub.io/status/lightgbm_3.2.1.99.tar.gz-0a4b82574ffe4f5b9172f7025fe87935
Reports also have been sent to LightGBM public e-mail: https://yopmail.com?lightgbm_rhub_checks
Status: success ✔️.

@david-cortes
Copy link
Contributor Author

The failing checks don't show any errors.

@jameslamb
Copy link
Collaborator

On https://github.com/microsoft/LightGBM/runs/3669905286?check_suite_focus=true, I see

Error: The operation was canceled.

And that the job runtime was 3 hours, which is the maximum we allow.

timeout-minutes: 180

This happens sometimes and is usually something transient related to the infrastructure behind GitHub Actions. I'll re-run the valgrind test.

@jameslamb
Copy link
Collaborator

jameslamb commented Sep 22, 2021

/gha run r-valgrind

Workflow R valgrind tests has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/1262796151

Status: success ✔️.

Copy link
Collaborator

@jameslamb jameslamb left a 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!

@jameslamb jameslamb changed the title [R-package] More memory safety [R-package] prevent memory leak if pointer fails to allocate Sep 22, 2021
@jameslamb jameslamb merged commit 75979ba into microsoft:master Sep 22, 2021
@github-actions
Copy link

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.

@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.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants