search

finos / perspective

A data visualization and analytics component, especially well-suited for large and/or streaming datasets.
https://perspective.finos.org/
Apache License 2.0
7.72k stars 1.04k forks source link

View::to_arrow_*() crashes when called from python #2597

Closed abalabin-bamfunds closed 2 months ago

abalabin-bamfunds commented 2 months ago

Bug Report

Steps to Reproduce:

  1. call View.to_arrow() from python

Expected Result:

it should work

Actual Result:

crash. when ran with debug-malloc python clearly states there were allocations attempted without GIL

Environment:

2.8.0/2.10.0, python 3.11/3.12, windows 10

Additional Context:

there is a bunch of methods that releases GIL to do stuff in C++ but does not grab it back before creating py::bytes object to return. See https://github.com/finos/perspective/compare/master...abalabin-bamfunds:perspective:fix-gil-unlocking?expand=1

texodus commented 2 months ago

Thanks for the report. This fix looks correct on sight read, do you want to open a PR? If not we can cherry-pick the changes from your fork.

abalabin-bamfunds commented 2 months ago

I created a PR, not sure if that's ok given the contribution doc says a CLA is required. If not, just cherry pick the commits

texodus commented 2 months ago

Apologies, the CLA process documentation was deleted without warning (externally hosted). We've just updated the project to a DCO contribution model, which only requires you to affirm your contribution by using the -s flag with git commit -s.

However, te PR also failed lint so I went ahead and re-authored this in #2608, this will be in Perspective v2.10.1

abalabin-bamfunds commented 2 months ago

Thanks, looking forward to the new version to be available. Will use DCO next time.