A few comments on the testsuite part of the patch.
First, I intend to test the patch using the attached pastebin script by
Trent, in addition to 'make check'.
Trent Nelson wrote on Mon, Jan 23, 2012 at 15:07:20 -0500:
> Index: subversion/bindings/swig/python/tests/mergeinfo.py
> --- subversion/bindings/swig/python/tests/mergeinfo.py (revision 1234786)
> +++ subversion/bindings/swig/python/tests/mergeinfo.py (working copy)
> + def test_mergeinfo_leakage__lingering_range_t_objects_after_del(self):
> + """Ensure that there are no svn_merge_range_t objects being tracked by
> + the garbage collector after we explicitly `del` the results returned
> + by svn_mergeinfo_parse(). We disable gc before the svn_mergeinfo_parse
> + call and force an explicit collection cycle straight after the `del`;
> + if our reference counts are correct, the allocated svn_merge_range_t
> + objects will be garbage collected and thus, not appear in the list of
> + objects returned by gc.get_objects()."""
> + gc.disable()
> + mergeinfo = core.svn_mergeinfo_parse(self.TEXT_MERGEINFO1)
> + del mergeinfo
> + gc.collect()
It seems you need a gc.enable() call as well, to prevent the test from
having effects on global state.
Do the swig-py tests support parallelized running? The testsuite for
'svn' does, and if the testsuite for swig-py does too then the 'gc' call
have effects on other threads.
> + lingering = get_svn_merge_range_t_objects()
> + self.assertEquals(lingering, list(), (
> + "Memory leak! Found lingering svn_merge_range_t objects left over from "
> + "our call to svn_mergeinfo_parse(), even though we explicitly deleted "
> + "the returned mergeinfo object. Probable cause: incorrect Py_INCREF/"
> + "Py_DECREF usage in libsvn_swig_py/swigutil_py.c. Lingering objects:\n"
> + "%s" % lingering))
Received on 2012-01-24 12:44:48 CET