Trent Nelson wrote on Tue, Jan 24, 2012 at 10:17:24 -0500: > On Tue, Jan 24, 2012 at 03:44:04AM -0800, Daniel Shahaf wrote: > > 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. > > You know what, you can get rid of that gc.disable() call. I put it > in by habit; when I was trying to find the leak, I had breakpoints > set within the CPython garbage collection routines -- if I didn't > disable automatic collection, they'd trigger at extremely annoying > times ;-) > > The gc.collect() call is sufficient. >
Okay. Given that, and that it seems a good idea to add a regression test for a bug that can cost many MB of memory if it repeats, I've committed the testsuite part too: r1235296, and will nominate for backport. > > 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. > > I have... no idea :-) Given that check-swig-py basically just calls > `python subversion/bindings/swig/python/tests/run_all.py', and that > file just uses the standard Python unit test facilities... I'd say > no, they're not currently designed to support parallel running. > > I'm sure we could totally shave that 40s test run time down a few > notches if they, though ;-) > I think the ROI will be better on optimizing the test suite of 'svn' itself (build/run_tests.py and subversion/tests/cmdline/) :-) > Trent. > Thanks, Daniel
