Describe the bug, including details regarding any error messages, version, and platform.
While reviewing:
I started investigating and I found that we could leak a reference and the items contained on the list when converting an iterator that raises an Exception at ConvertToSequenceAndInferSize.
I created a test that currently fails:
def test_failing_iterator_does_not_leak():
# ConvertToSequenceAndInferSize builds an intermediate list. If the
# iterator raises, that list and everything already stored in it leaks.
import gc
# Create an arbitrary long int that is probably not cached by the interpreter
value = 10**20
def raising_iter():
for _ in range(5):
yield value
raise ValueError("boom")
gc.collect()
original_refcount = sys.getrefcount(value)
with pytest.raises(ValueError, match="boom"):
pa.array(raising_iter(), size=100)
gc.collect()
assert sys.getrefcount(value) == original_refcount
The issue is in RETURN_IF_PYERROR(); error case where lst is not Py_DECREF:
|
PyObject* lst = PyList_New(n); |
|
RETURN_IF_PYERROR(); |
|
for (i = 0; i < n; i++) { |
|
PyObject* item = PyIter_Next(iter); |
|
if (!item) { |
|
// either an error occurred or the iterator ended |
|
RETURN_IF_PYERROR(); |
|
break; |
|
} |
|
PyList_SET_ITEM(lst, i, item); |
|
} |
|
// Shrink list if len(iterator) < size |
|
if (i < n && PyList_SetSlice(lst, i, n, NULL)) { |
|
Py_DECREF(lst); |
|
RETURN_IF_PYERROR(); |
|
} |
|
*seq = lst; |
The failing iterator case was detected here:
Component(s)
Python
Describe the bug, including details regarding any error messages, version, and platform.
While reviewing:
I started investigating and I found that we could leak a reference and the items contained on the list when converting an iterator that raises an Exception at
ConvertToSequenceAndInferSize.I created a test that currently fails:
The issue is in
RETURN_IF_PYERROR();error case where lst is notPy_DECREF:arrow/python/pyarrow/src/arrow/python/python_to_arrow.cc
Lines 1260 to 1276 in 154962c
The failing iterator case was detected here:
Component(s)
Python