Author: svn-role
Date: Tue Feb  7 04:00:08 2012
New Revision: 1241344

URL: http://svn.apache.org/viewvc?rev=1241344&view=rev
Log:
Merge the r1235264 group from trunk:

 * r1235264, r1235296, r1235302, r1235736
   Fix a swig-py memory leak.
   Justification:
     Seen in the wild with significant effects.
   Notes:
     r1235264 is the fix.
     r1235296 adds a regression test.
     r1235302 is trivial / obvious fix.
     r1235736 is trivial / obvious fix.
   Votes:
     +1: danielsh
     +0: gstein

Modified:
    subversion/branches/1.6.x/   (props changed)
    subversion/branches/1.6.x/STATUS
    
subversion/branches/1.6.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
    subversion/branches/1.6.x/subversion/bindings/swig/python/tests/mergeinfo.py

Propchange: subversion/branches/1.6.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Feb  7 04:00:08 2012
@@ -121,4 +121,4 @@
 /subversion/branches/tc_url_rev:874351-874483
 /subversion/branches/tree-conflicts:868291-873154
 /subversion/branches/tree-conflicts-notify:873926-874008
-/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876024,876032,876041-876042,876048,876051,876055-876056,876059,876083,876091,876097,876101,876104,876109,876123-876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,876223-876225,876230,876233,876245,876252,876256,876283,876287,876312,876326-876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876601,876614-876615,876628,876633,876641,876645,876659,876687,876689,876705,876715,876726,876760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,876843,876849,876857-876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,876948-876949,876953,876987,876993,877011,877014,877016,877028-877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,877230,877234,877237,877243,877249,877259,877261,877304,877319,877407,877437,877441-877442,877453,87745
 
9,877472,877544,877553,877565,877568,877573,877593,877595,877597,877601,877612,877665,877667,877681,877692,877696,877701,877720,877730,877784,877793,877797,877809,877814-877815,877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,877942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129,878131,878142,878173-878176,878216,878240,878242,878255,878269,878272,878279,878296-878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-878368,878385,878399,878423,878426,878447,878462,878484,878491,878498,878532,878590,878595,878607,878625-878627,878646,878659,878673,878682-878683,878690-878691,878693,878723,878760-878761,878873,878875,878877,878879,878905,878910-878911,878915-878916,878924-878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,879076,879081-879082,879093,879105,879126,879148,879170,879198-879199,879201,879271,
 
879293,879357,879375-879376,879403,879631,879635-879636,879688,879709-879711,879747,879902,879916,879954,879961,879966,879971,880027,880082,880095,880105,880146,880162,880226,880274-880275,880370,880450,880461,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,893478,895514,895653,896522,896915,898048,898963,899826,899828,900797,901304,901752,902093,902467,904301,904394,904594,905303,905326,906256,906305,906587,907644,908980-908981,917523,917640,918211,922516,923389,923391,926151,926167,927323,927328,931209,931211,931392,931568,932942,933299,934599,934603,935631,935992,935996,937610,939000,939002,939375-939376,944635,945350,946355,946767,947006,948512,948916,949307,950931,950933,951753,952992,953317,955369,957507,958024,959004,959760,961055,961970,962377-962378,964167,964349,964767,965405,965469,965508,979045,979429,980811,981449,981921,984565,984928,984931,991534,992114,996884,997026,997070,997457,997466,997471,997474,10
 
00038,1000060,1000607,1000612,1001009,1002094,1005446,1022675,1024269,1027957,1028084,1028108,1028125,1031165,1031186,1032808,1033166,1033290,1033665,1033685,1033921,1034557,1035745,1036429,1036534,1036978,1037762,1038792,1039040,1041438,1041638,1051632,1051638,1051733,1051744-1051745,1051751,1051761,1051763,1051775,1051778,1051968,1051978,1051988,1052029,1052041,1052068,1053185,1053208,1053233,1053499,1053984,1058269,1058722,1063572-1063573,1063592,1063870,1063946,1064839,1066249,1066270,1066276,1068988,1070912,1071239,1071307,1072084,1072953,1074572,1076730,1076759,1076826,1078954,1081255,1084575,1084581,1084764,1084962,1084978,1086222,1094692,1095654,1098608,1102803,1103665,1104309,1125983,1125998,1126007,1126810,1130303,1130448,1134734,1144316,1146870,1151036,1240752
+/subversion/trunk:875965,875968,876004,876012,876017,876019,876022,876024,876032,876041-876042,876048,876051,876055-876056,876059,876083,876091,876097,876101,876104,876109,876123-876125,876129,876132,876138,876160,876167,876175,876180,876185,876205,876223-876225,876230,876233,876245,876252,876256,876283,876287,876312,876326-876327,876330,876366,876372,876374,876376,876383,876386,876442,876456-876457,876462-876464,876467,876469,876480,876486,876495-876497,876516-876518,876524,876526,876583,876601,876614-876615,876628,876633,876641,876645,876659,876687,876689,876705,876715,876726,876760,876763,876794,876804,876815-876816,876821,876825,876837,876840-876841,876843,876849,876857-876858,876862,876873,876890,876897,876905,876908,876925,876931,876934,876948-876949,876953,876987,876993,877011,877014,877016,877028-877029,877038,877119,877127,877146,877157,877191,877195,877203,877211,877230,877234,877237,877243,877249,877259,877261,877304,877319,877407,877437,877441-877442,877453,87745
 
9,877472,877544,877553,877565,877568,877573,877593,877595,877597,877601,877612,877665,877667,877681,877692,877696,877701,877720,877730,877784,877793,877797,877809,877814-877815,877819,877821,877842,877848,877853,877867,877869,877873,877901,877909,877916,877931,877942,877953,877964,877968,877970,877981-877982,878005,878013,878015,878020,878046,878053,878062,878074,878080,878089,878091,878093,878095,878127,878129,878131,878142,878173-878176,878216,878240,878242,878255,878269,878272,878279,878296-878297,878303,878321,878335,878338,878341,878343,878353,878364,878367-878368,878385,878399,878423,878426,878447,878462,878484,878491,878498,878532,878590,878595,878607,878625-878627,878646,878659,878673,878682-878683,878690-878691,878693,878723,878760-878761,878873,878875,878877,878879,878905,878910-878911,878915-878916,878924-878925,878946,878949,878955,878960,878970,878981,879001,879033,879056,879074,879076,879081-879082,879093,879105,879126,879148,879170,879198-879199,879201,879271,
 
879293,879357,879375-879376,879403,879631,879635-879636,879688,879709-879711,879747,879902,879916,879954,879961,879966,879971,880027,880082,880095,880105,880146,880162,880226,880274-880275,880370,880450,880461,880474,880525-880526,880552,881905,884842,886164,886197,888715,888979,889081,889840,891672,892050,892085,893478,895514,895653,896522,896915,898048,898963,899826,899828,900797,901304,901752,902093,902467,904301,904394,904594,905303,905326,906256,906305,906587,907644,908980-908981,917523,917640,918211,922516,923389,923391,926151,926167,927323,927328,931209,931211,931392,931568,932942,933299,934599,934603,935631,935992,935996,937610,939000,939002,939375-939376,944635,945350,946355,946767,947006,948512,948916,949307,950931,950933,951753,952992,953317,955369,957507,958024,959004,959760,961055,961970,962377-962378,964167,964349,964767,965405,965469,965508,979045,979429,980811,981449,981921,984565,984928,984931,991534,992114,996884,997026,997070,997457,997466,997471,997474,10
 
00038,1000060,1000607,1000612,1001009,1002094,1005446,1022675,1024269,1027957,1028084,1028108,1028125,1031165,1031186,1032808,1033166,1033290,1033665,1033685,1033921,1034557,1035745,1036429,1036534,1036978,1037762,1038792,1039040,1041438,1041638,1051632,1051638,1051733,1051744-1051745,1051751,1051761,1051763,1051775,1051778,1051968,1051978,1051988,1052029,1052041,1052068,1053185,1053208,1053233,1053499,1053984,1058269,1058722,1063572-1063573,1063592,1063870,1063946,1064839,1066249,1066270,1066276,1068988,1070912,1071239,1071307,1072084,1072953,1074572,1076730,1076759,1076826,1078954,1081255,1084575,1084581,1084764,1084962,1084978,1086222,1094692,1095654,1098608,1102803,1103665,1104309,1125983,1125998,1126007,1126810,1130303,1130448,1134734,1144316,1146870,1151036,1235264,1235296,1235302,1235736,1240752

Modified: subversion/branches/1.6.x/STATUS
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.6.x/STATUS?rev=1241344&r1=1241343&r2=1241344&view=diff
==============================================================================
--- subversion/branches/1.6.x/STATUS (original)
+++ subversion/branches/1.6.x/STATUS Tue Feb  7 04:00:08 2012
@@ -269,16 +269,3 @@ Veto-blocked changes:
 
 Approved changes:
 =================
-
- * r1235264, r1235296, r1235302, r1235736
-   Fix a swig-py memory leak.
-   Justification:
-     Seen in the wild with significant effects.
-   Notes:
-     r1235264 is the fix.
-     r1235296 adds a regression test.
-     r1235302 is trivial / obvious fix.
-     r1235736 is trivial / obvious fix.
-   Votes:
-     +1: danielsh
-     +0: gstein

Modified: 
subversion/branches/1.6.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c?rev=1241344&r1=1241343&r2=1241344&view=diff
==============================================================================
--- 
subversion/branches/1.6.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
 (original)
+++ 
subversion/branches/1.6.x/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
 Tue Feb  7 04:00:08 2012
@@ -652,10 +652,22 @@ static PyObject *convert_rangelist(void 
   apr_array_header_t *array = value;
 
   list = PyList_New(0);
+  if (list == NULL)
+    return NULL;
+
   for (i = 0; i < array->nelts; i++)
     {
       svn_merge_range_t *range = APR_ARRAY_IDX(array, i, svn_merge_range_t *);
-      if (PyList_Append(list, convert_to_swigtype(range, ctx, py_pool)) == -1)
+      PyObject *obj;
+      int result;
+
+      obj = convert_to_swigtype(range, ctx, py_pool);
+      if (obj == NULL)
+        goto error;
+
+      result = PyList_Append(list, obj);
+      Py_DECREF(obj);
+      if (result == -1)
         goto error;
     }
   return list;

Modified: 
subversion/branches/1.6.x/subversion/bindings/swig/python/tests/mergeinfo.py
URL: 
http://svn.apache.org/viewvc/subversion/branches/1.6.x/subversion/bindings/swig/python/tests/mergeinfo.py?rev=1241344&r1=1241343&r2=1241344&view=diff
==============================================================================
--- 
subversion/branches/1.6.x/subversion/bindings/swig/python/tests/mergeinfo.py 
(original)
+++ 
subversion/branches/1.6.x/subversion/bindings/swig/python/tests/mergeinfo.py 
Tue Feb  7 04:00:08 2012
@@ -1,4 +1,4 @@
-import unittest, os
+import unittest, os, sys, gc
 
 from sys import version_info # For Python version check
 if version_info[0] >= 3:
@@ -23,6 +23,15 @@ class RevRange:
     self.start = start
     self.end = end
 
+def get_svn_merge_range_t_objects():
+  """Returns a list 'svn_merge_range_t' objects being tracked by the
+     garbage collector, used for detecting memory leaks."""
+  return [
+    o for o in gc.get_objects()
+      if hasattr(o, '__class__') and
+        o.__class__.__name__ == 'svn_merge_range_t'
+  ]
+
 class SubversionMergeinfoTestCase(unittest.TestCase):
   """Test cases for mergeinfo"""
 
@@ -114,6 +123,53 @@ class SubversionMergeinfoTestCase(unitte
       }
     self.compare_mergeinfo_catalogs(mergeinfo, expected_mergeinfo)
 
+  def test_mergeinfo_leakage__incorrect_range_t_refcounts(self):
+    """Ensure that the ref counts on svn_merge_range_t objects returned by
+       svn_mergeinfo_parse() are correct."""
+    # When reference counting is working properly, each svn_merge_range_t in
+    # the returned mergeinfo will have a ref count of 1...
+    mergeinfo = core.svn_mergeinfo_parse(self.TEXT_MERGEINFO1)
+    for (path, rangelist) in mergeinfo.items():
+      # ....and now 2 (incref during iteration of rangelist)
+
+      for (i, r) in enumerate(rangelist):
+        # ....and now 3 (incref during iteration of each range object)
+
+        refcount = sys.getrefcount(r)
+        # ....and finally, 4 (getrefcount() also increfs)
+        expected = 4
+
+        # Note: if path and index are not '/trunk' and 0 respectively, then
+        # only some of the range objects are leaking, which is, as far as
+        # leaks go, even more impressive.
+        self.assertEquals(refcount, expected, (
+          "Memory leak!  Expected a ref count of %d for svn_merge_range_t "
+          "object, but got %d instead (path: %s, index: %d).  Probable "
+          "cause: incorrect Py_INCREF/Py_DECREF usage in libsvn_swig_py/"
+          "swigutil_py.c." % (expected, refcount, path, i)))
+
+    del mergeinfo
+    gc.collect()
+
+  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 call gc.collect() to force an explicit
+       garbage collection cycle 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()."""
+    mergeinfo = core.svn_mergeinfo_parse(self.TEXT_MERGEINFO1)
+    del mergeinfo
+    gc.collect()
+    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))
+
   def inspect_mergeinfo_dict(self, mergeinfo, merge_source, nbr_rev_ranges):
     rangelist = mergeinfo.get(merge_source)
     self.inspect_rangelist_tuple(rangelist, nbr_rev_ranges)


Reply via email to