[c++][python] Possibly memory not deallocated when reading in CSV

[asfimport]

When one reads in a table from CSV in pyarrow version 4.0.1, it appears that the read-in table variable is not freed (or not fast enough). I'm unsure if this is because of pyarrow or because of the way pyarrow memory allocation interacts with Python memory allocation. I encountered it when processing many large CSVs sequentially.

When I run the following piece of code, the RAM memory usage increases quite rapidly until it runs out of memory.

import pyarrow as pa
import pyarrow.csv

# Generate some CSV file to read in
print("Generating CSV")
with open("example.csv", "w+") as f_out:
    for i in range(0, 10000000):
        f_out.write("123456789,abc def ghi jkl\n")


def read_in_the_csv():
    table = pa.csv.read_csv("example.csv")
    print(table)  # Not strictly necessary to replicate bug, table can also be an unused variable
    # This will free up the memory, as a workaround:
    # table = table.slice(0, 0)


# Read in the CSV many times
print("Reading in a CSV many times")
for j in range(100000):
    read_in_the_csv()

Reporter: Simon
Assignee: Antoine Pitrou / @pitrou

Original Issue Attachments:

• backward-refs.png
• forward-refs.png

PRs and other links:

• GitHub Pull Request #10609

_{Note: This issue was originally created as ARROW-13187. Please see the migration documentation for further details.}

[asfimport]

Weston Pace / @westonpace:
Great reproduction, thank you.  I can reproduce this on 4.0.0 but not on 3.0.0.  A few observations so far:

pa.total_allocated_bytes is increasing so it is not a dynamic allocator blowup issue.

"del table" prevents the out-of-ram (same as the table.slice above).

"gc.collect" prevents the out-of-ram

 

Those workarounds shouldn't be necessary however.  When read_in_the_csv exits the table is no longer needed, it's refcount should decrease by 1, and it should be eligible for garbage collection.  Combined with the fact that this doesn't occur on 3.0.0 (both environments are using python 3.8 although 3.8.6 vs 3.8.8 but I doubt it's a python change) I think this means that a circular reference was introduced in the Arrow->Python code between 3.0.0 and 4.0.0.

[asfimport]

Weston Pace / @westonpace:
Also, it seems this does not happen when repeatedly reading in a parquet file.  So maybe it isn't in the Arrow->Python code or maybe it's particular to the way the CSV reader is creating the table.

[asfimport]

Weston Pace / @westonpace:
I have tracked down the cause further but I'm not entirely sure what the correct fix should be but I think it is a problem in cython.  The issue first occurs after commit 79ae4f6 (ARROW-8732: [C++] Add basic cancellation API)

 

The method "read_csv" calls "SignalStopHandler()" which calls "signal.getsignal" which calls "signal.py::_int_to_enum" which intentionally triggers a ValueError (as is normal in python).

 

That ValueError has an associated traceback which is not disposed of correctly.  That traceback has a reference to each of the frames of the stack and one of those frames has a reference to "table".  Since a new traceback is generated for each loop of the iteration none of the CSVs are properly disposed of.  The slice method in the original PR or "del table" is a workable workaround.  As long as the frames aren't too big the garbage collector will eventually run and clean them up long before memory is lost.

 

I have no idea why the ValueError/traceback are not being disposed of.  I know cython has to do some games to manage tracebacks so it's possible there is an issue there.  I think I created a reproduction in pure python calling getsignal and it seems to manage memory correctly so I believe python is clear.

 

I've created a script to reproduce that also uses objgraph to generate reference graphs.  It also only runs one iteration so it is quicker and doesn't exceed RAM on the system.  It should print 0 as the last line.  If there is a leak it prints out ~270M.

 

 

import gc
import sys
import pyarrow.parquet
import pyarrow as pa
import pyarrow.csv
import objgraph

# Generate some CSV file to read in                                                                                                                                                                                
print("Generating CSV")
with open("example.csv", "w+") as f_out:
    for i in range(0, 10000000):
        unused = f_out.write("123456789,abc def ghi jkl\n")

def read_in_the_csv():
    table = pa.csv.read_csv("example.csv")
    print(pa.total_allocated_bytes())

gc.disable()
gc.collect()
objs = gc.get_objects()
read_in_the_csv()
objs2 = gc.get_objects()
offensive_ids = set([id(obj) for obj in objs2]) - set([id(obj) for obj in objs])
badobjs = [obj for obj in objs2 if id(obj) in offensive_ids]
print(len(badobjs))
smallbadobjs = [obj for obj in badobjs if 'frame' in str(type(obj)) and 'read_in_the_csv' in str(obj)]
objgraph.show_refs(smallbadobjs, refcounts=True)
objgraph.show_backrefs(smallbadobjs, refcounts=True)
print(pa.total_allocated_bytes())

 

So at this point I surrender and ask @pitrou @jorisvandenbossche or @amol- for help :)

 

Forward refs show a frame in the traceback still reference Table

Backward refs show the frame is referenced as part of a traceback (note, this graph is truncated, and does not show the source ValueError.  Also, the dict and two lists are from my debugging code and not related to the issue)

 

 

[asfimport]

Antoine Pitrou / @pitrou:
This seems to be, well, a classical cyclic reference issue due to a traceback. It's trivially reproducible on a Python prompt:

>>> import signal, gc, weakref
>>> gc.disable()   # disable automatic cyclic reference collection
>>> class C: pass
... 
>>> def h(*args): pass
... 
>>> signal.signal(signal.SIGINT, h)
<built-in function default_int_handler>
>>> 
>>> def f():
...   global wr
...   c = C()
...   wr = weakref.ref(c)
...   signal.getsignal(signal.SIGINT)
... 
>>> f()
>>> wr() is None
False    # object `c` is still alive
>>> gc.collect()    # collect cyclic references
17
>>> wr() is None
True     # object `c` has been collected

[asfimport]

Antoine Pitrou / @pitrou:
This was fixed upstream in https://bugs.python.org/issue42248 . You'll get the fix in Python 3.8.10 and 3.9.5.

 

 

[asfimport]

Simon:
Thank you for the prompt help. I'll for now then stick with the workaround.

As a sidenote, it seems it might take quite time before Python 3.8.10 is in use by the major distributions (with e.g., Ubuntu currently at 3.8.5).
Given that pyarrow is used for "large objects" as referred to in https://bugs.python.org/issue42248 by Gerald Dalley, it might be worthwhile to have it noted somewhere in pyarrow documentation till Python is upgraded more broadly? As reading in many large CSVs in sequence could be considered a common task.

[asfimport]

Weston Pace / @westonpace:
I agree it would be nice to avoid this given the amount of RAM that could be getting held onto.  Some crude potential workarounds off the top of my head:

 * Create a new thread to run the getsignal check

 * manually run gc.collect after the getsignal check

 * implement the getsignal check ourselves in C++

 

@pitrou if any of those workarounds sound acceptable (or if you have a better idea) I'm happy to implement it.

[asfimport]

Antoine Pitrou / @pitrou:
I can try to cook a workaround.

[asfimport]

Antoine Pitrou / @pitrou:
Issue resolved by pull request 10609
#10609