ICRAR / ijson / 298

100%

push

travis-ci-com

Avoid decref'ing the sink argument

In #34 memory leaks were reported on the yajl2_c backend that happened
at generator construction time. After investigation, we discovered one
of these in the "chain" utility method, which leaked the temporary
coroutines it chained together. The fix in a8159e4 (first present in
3.1.2) attempted to fix this particular problem, along with the rest of
the identified memory leaks.

The fix for the "chain" method was not correct though: the condition we
used to identify whether the internal "coro" variable had to be decref'd
was incorrect, and led to our code incorrect decref'ing the "sink"
parameter given by the caller, which was meant to be left untouched.
This opened the door to potential issues when the caller decref'd its
reference to "sink".

The problem was not immediately obvious, as depending on the program's
activity, memory allocator and other factors, the memory used by the
destroyed "sink" object could still be valid. In fact our unit tests
never caught this error, and this problem was only reported to us about
a year after 3.1.2 was released by users who saw this problem only after
creating and destroying ijson.parser objects thousands of times.

To reproduce this issue locally I wrote a small script that created a
list of N ijson.parser generators, and then depleted them in order. Only
when running with N=10k I was able to get a failure every 2 or 3
executions.  After applying this patch I saw no failures, so I'm fairly
confident the issue is gone.

This problem was reported originally in #66, which this commit should
fix.

Signed-off-by: Rodrigo Tobar <rtobar@icrar.org>

1 of 1 new or added line in 1 file covered. (100.0%)

1515 of 1518 relevant lines covered (99.8%)

420.67 hits per line