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.

Sign up for GitHub

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

Jump to bottom

Fix #6130 objmode cache segfault #6141

Merged
merged 9 commits into from Aug 25, 2020
Merged

Fix #6130 objmode cache segfault #6141

merged 9 commits into from Aug 25, 2020

Conversation

sklam
Copy link
Member

@sklam sklam commented Aug 17, 2020

Fixes #6130

Thanks @juanjgalvez and @stuartarchibald for finding out the source of the problem on the lost env.

The patch makes sure the CompileResult acquires a reference to any referenced Environment objects. Changes to numba/core/compiler.py is the actual fix. The other non-test changes are for better error handling when an NULL pointer is encountered.

@sklam sklam added this to the Numba 0.51.1 milestone Aug 17, 2020
stuartarchibald
stuartarchibald reviewed Aug 20, 2020
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patch, great to see this fixed.

numba/tests/support.py Show resolved Hide resolved
numba/core/runtime/_nrt_python.c Outdated Show resolved Hide resolved
numba/core/pythonapi.py Outdated Show resolved Hide resolved
numba/core/pythonapi.py Outdated Show resolved Hide resolved
numba/tests/support.py Outdated Show resolved Hide resolved
numba/tests/test_extending.py Show resolved Hide resolved
numba/tests/test_extending.py Show resolved Hide resolved
numba/tests/test_extending.py Show resolved Hide resolved
@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Aug 20, 2020
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Aug 20, 2020
@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Aug 21, 2020
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Aug 24, 2020
stuartarchibald
stuartarchibald reviewed Aug 25, 2020
View reviewed changes
numba/_helperlib.c Outdated Show resolved Hide resolved
@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Aug 25, 2020
@sklam
Apply suggestion from code review
62d2a8b 
- changed to RuntimeError and fix error message
- added suggested comment
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Aug 25, 2020
stuartarchibald
stuartarchibald reviewed Aug 25, 2020
View reviewed changes
numba/_helperlib.c Outdated Show resolved Hide resolved
@sklam
Copy link
Member Author

sklam commented Aug 25, 2020

failing due to: #6168

@stuartarchibald
Copy link
Contributor

/AzurePipelines run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

stuartarchibald
stuartarchibald approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge 4 - Waiting on CI Review etc done, waiting for CI to finish and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author 5 - Ready to merge Review and testing done, is ready to merge labels Aug 25, 2020
@sklam sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on CI Review etc done, waiting for CI to finish labels Aug 25, 2020
@sklam
Copy link
Member Author

sklam commented Aug 25, 2020

CI passed, marked for merge

@sklam sklam merged commit 516ec41 into numba:master Aug 25, 2020
@stuartarchibald stuartarchibald mentioned this pull request Aug 26, 2020
Closed
21 tasks
stuartarchibald pushed a commit to stuartarchibald/numba that referenced this pull request Aug 26, 2020
@sklam @stuartarchibald
Merge pull request numba#6141 from sklam/fix/objmode_cache_segfault
4f248d3 
Fix numba#6130 objmode cache segfault
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser left review comments

@stuartarchibald stuartarchibald stuartarchibald approved these changes

Assignees
No one assigned
Labels
5 - Ready to merge Review and testing done, is ready to merge
Projects
None yet
Milestone
Numba 0.51.1
Development

Successfully merging this pull request may close these issues.

Cache does not work when passing numpy arrays to objmode inside overload
3 participants
@sklam @stuartarchibald @eric-wieser