Skip to content

Commit 911cb18

Browse files
committed
[Python] Don't rely on possible memory leaks in T(Seq)Collection tests
Some unit tests rely on the behavior that when adding object to `TLists`, Python ownership is always dropped, resulting in memory leaks if the ownership is not correctly handled on the C++ side. To make these unit tests transparently ready for the moment where the ownership is handled correctly, this commit suggests to use some global lists that keep around Python references until the end of the test. So the effect on the lifetime is the same as having the "controlled" memory leaks.
1 parent f6afc4d commit 911cb18

3 files changed

Lines changed: 22 additions & 7 deletions

File tree

bindings/pyroot/pythonizations/test/tcollection_listmethods.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,16 @@ class TCollectionListMethods(unittest.TestCase):
1111

1212
num_elems = 3
1313

14+
_global_objects = []
15+
1416
# Helpers
1517
def create_tcollection(self):
1618
c = ROOT.TList()
1719
for _ in range(self.num_elems):
1820
o = ROOT.TObject()
19-
# Prevent immediate deletion of C++ TObjects
20-
ROOT.SetOwnership(o, False)
2121
c.Add(o)
22+
# To prevent deletion of the objects (TList is by default non-owning)
23+
self._global_objects.append(o)
2224

2325
return c
2426

bindings/pyroot/pythonizations/test/tseqcollection_itemaccess.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ class TSeqCollectionItemAccess(unittest.TestCase):
1212

1313
num_elems = 3
1414

15+
_global_objects = []
16+
1517
# Helpers
1618
def create_tseqcollection(self):
1719
sc = ROOT.TList()
1820
for _ in range(self.num_elems):
1921
o = ROOT.TObject()
20-
# Prevent immediate deletion of C++ TObjects
21-
ROOT.SetOwnership(o, False)
2222
sc.Add(o)
23+
# To prevent deletion of the objects (TList is by default non-owning)
24+
self._global_objects.append(o)
2325

2426
return sc
2527

bindings/pyroot/pythonizations/test/tseqcollection_listmethods.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,16 @@ class TSeqCollectionListMethods(unittest.TestCase):
1111

1212
num_elems = 3
1313

14+
_global_objects = []
15+
1416
# Helpers
1517
def create_tseqcollection(self):
1618
sc = ROOT.TList()
1719
for i in reversed(range(self.num_elems)):
18-
sc.Add(ROOT.TObjString(str(i)))
20+
o = ROOT.TObjString(str(i))
21+
sc.Add(o)
22+
# To prevent deletion of the objects (TList is by default non-owning)
23+
self._global_objects.append(o)
1924

2025
return sc
2126

@@ -49,6 +54,10 @@ def test_insert(self):
4954
self.assertEqual(sc.GetEntries(), self.num_elems + 4)
5055
self.assertEqual(sc.At(self.num_elems + 3), o4)
5156

57+
# Clear before the added element might be garbage collected,
58+
# to avoid dangling pointer access.
59+
sc.Clear()
60+
5261
def test_pop(self):
5362
sc = self.create_tseqcollection()
5463
l1 = [elem for elem in sc]
@@ -82,8 +91,10 @@ def test_pop(self):
8291
with self.assertRaises(TypeError):
8392
sc2.pop(1.0)
8493

85-
# Pop a repeated element
86-
sc2.append(ROOT.TObjString("2"))
94+
# Pop a repeated element.
95+
# Keep Python reference so the added element lives beyond the sc2.pop() call:
96+
new_elem = ROOT.TObjString("2")
97+
sc2.append(new_elem)
8798
elem = sc2.pop()
8899
self.assertEqual(sc2.At(0), elem)
89100

0 commit comments

Comments
 (0)