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

Make KNN cython object use __dealloc__ instead of __del__ #4831

Closed
wants to merge 2 commits into from

Conversation

achirkin
Copy link
Contributor

The new code in rapidsai/raft#652 makes cython NearestNeighbors wrapper segfault during deallocation / garbage collection when trying to destruct mdarray members of ivf_flat::index.
The reason for this may be way cython handles __del__ vs __dealloc__, thus we replace here the former with the latter.

@achirkin achirkin requested review from a team as code owners July 26, 2022 14:11
@achirkin achirkin marked this pull request as draft July 26, 2022 14:11
@github-actions github-actions bot added CMake CUDA/C++ Cython / Python Cython or Python issue labels Jul 26, 2022
@achirkin achirkin added bug Something isn't working non-breaking Non-breaking change Cython / Python Cython or Python issue and removed Cython / Python Cython or Python issue CMake CUDA/C++ labels Jul 26, 2022
@achirkin
Copy link
Contributor Author

Closing this, because the solution is incorrect: NearestNeighbors is not a cython extension type, and thus simply doesn't call __dealloc__.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CMake CUDA/C++ Cython / Python Cython or Python issue non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant