ICRAR / ijson

100%

master: 100%

LAST BUILD ON BRANCH issue-66

branch: issue-66

CHANGE BRANCH

x

Reset

• issue-66
• async-fixes
• async-string-stream
• async_c
• bytes-prefix
• check-3.1.2.post0
• common_functions
• coros
• coverage-c
• coverage-increase
• decimal-c
• decoding-errors
• default_backend
• design_notes
• develop
• document-event-interception
• dump
• empty-members
• fixes-for-known-problems
• gh-actions
• ijson-dumps-multiple
• improved-readme
• issue-41
• issue-42
• items-and-kvitems-dont-join-path
• items_kwargs
• kvitems
• map_type
• master
• memory-leak
• more_tests
• new-devels
• no-tests-in-binary
• python-3.10
• python-parse-value-clarification
• python_multiple_values
• reader_conversion
• release-3.2.0
• rtobar-patch-1
• test
• test-comments
• test-module-rename
• test-wheels-on-travis
• toplevel_array
• travis-c-ext
• travis-more-python
• travis-xenial
• update-travis
• upload-check
• use_float
• v2.5
• v2.5.1
• v2.6.0
• v2.6.1
• v3.0
• v3.0.1
• v3.0.2
• v3.0.3
• v3.0.4
• v3.0rc1
• v3.0rc2
• v3.0rc3
• v3.1
• v3.1.1
• v3.1.2
• v3.1.2.post0
• v3.1.3
• v3.1.4
• v3.1.post0
• windows-wheels
• yajl109_test
• yajl2_c_parse_memleak
• zeros-python

Build # 294

Build Type

push

travis-ci-com

Committed by rtobar

Commit Message

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>

Run Details

893 of 894 relevant lines covered (99.89%)

8.49 hits per line