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

Catch all exceptions in Julia #3304

Merged
merged 3 commits into from Nov 21, 2022

Conversation

rcurtin
Copy link
Member

@rcurtin rcurtin commented Nov 3, 2022

Ideally, when running an mlpack binding from Julia, we want to catch all exceptions, instead of crashing.

A typo in the existing code meant we only caught std::runtime_errors instead of any exception. So, easy fix: just catch std::exception instead.

Note that for these to be caught correctly, the Julia bindings should not be compiled with -DARMA_NO_DEBUG, otherwise the out-of-bounds checks will not be done and may cause invalid memory accesses and segfaults, instead of throwing exceptions.

Copy link
Member

@shrit shrit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, is anything else required to be added to this one?

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second approval provided automatically after 24 hours. 👍

@conradsnicta conradsnicta merged commit d448bbb into mlpack:master Nov 21, 2022
@rcurtin rcurtin mentioned this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants