Skip to content

Commit c31b8a9

Browse files
sweeneydesweeneyde
authored
bpo-46615: Don't crash when set operations mutate the sets (GH-31120) (GH-31312)
Ensure strong references are acquired whenever using `set_next()`. Added randomized test cases for `__eq__` methods that sometimes mutate sets when called. (cherry picked from commit 4a66615)
1 parent ebe73e6 commit c31b8a9

File tree

3 files changed

+226
-8
lines changed

3 files changed

+226
-8
lines changed

‎Lib/test/test_set.py

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1799,6 +1799,192 @@ def __eq__(self, o):
17991799
s = {0}
18001800
s.update(other)
18011801

1802+
1803+
class TestOperationsMutating:
1804+
"""Regression test for bpo-46615"""
1805+
1806+
constructor1 = None
1807+
constructor2 = None
1808+
1809+
def make_sets_of_bad_objects(self):
1810+
class Bad:
1811+
def __eq__(self, other):
1812+
if not enabled:
1813+
return False
1814+
if randrange(20) == 0:
1815+
set1.clear()
1816+
if randrange(20) == 0:
1817+
set2.clear()
1818+
return bool(randrange(2))
1819+
def __hash__(self):
1820+
return randrange(2)
1821+
# Don't behave poorly during construction.
1822+
enabled = False
1823+
set1 = self.constructor1(Bad() for _ in range(randrange(50)))
1824+
set2 = self.constructor2(Bad() for _ in range(randrange(50)))
1825+
# Now start behaving poorly
1826+
enabled = True
1827+
return set1, set2
1828+
1829+
def check_set_op_does_not_crash(self, function):
1830+
for _ in range(100):
1831+
set1, set2 = self.make_sets_of_bad_objects()
1832+
try:
1833+
function(set1, set2)
1834+
except RuntimeError as e:
1835+
# Just make sure we don't crash here.
1836+
self.assertIn("changed size during iteration", str(e))
1837+
1838+
1839+
class TestBinaryOpsMutating(TestOperationsMutating):
1840+
1841+
def test_eq_with_mutation(self):
1842+
self.check_set_op_does_not_crash(lambda a, b: a == b)
1843+
1844+
def test_ne_with_mutation(self):
1845+
self.check_set_op_does_not_crash(lambda a, b: a != b)
1846+
1847+
def test_lt_with_mutation(self):
1848+
self.check_set_op_does_not_crash(lambda a, b: a < b)
1849+
1850+
def test_le_with_mutation(self):
1851+
self.check_set_op_does_not_crash(lambda a, b: a <= b)
1852+
1853+
def test_gt_with_mutation(self):
1854+
self.check_set_op_does_not_crash(lambda a, b: a > b)
1855+
1856+
def test_ge_with_mutation(self):
1857+
self.check_set_op_does_not_crash(lambda a, b: a >= b)
1858+
1859+
def test_and_with_mutation(self):
1860+
self.check_set_op_does_not_crash(lambda a, b: a & b)
1861+
1862+
def test_or_with_mutation(self):
1863+
self.check_set_op_does_not_crash(lambda a, b: a | b)
1864+
1865+
def test_sub_with_mutation(self):
1866+
self.check_set_op_does_not_crash(lambda a, b: a - b)
1867+
1868+
def test_xor_with_mutation(self):
1869+
self.check_set_op_does_not_crash(lambda a, b: a ^ b)
1870+
1871+
def test_iadd_with_mutation(self):
1872+
def f(a, b):
1873+
a &= b
1874+
self.check_set_op_does_not_crash(f)
1875+
1876+
def test_ior_with_mutation(self):
1877+
def f(a, b):
1878+
a |= b
1879+
self.check_set_op_does_not_crash(f)
1880+
1881+
def test_isub_with_mutation(self):
1882+
def f(a, b):
1883+
a -= b
1884+
self.check_set_op_does_not_crash(f)
1885+
1886+
def test_ixor_with_mutation(self):
1887+
def f(a, b):
1888+
a ^= b
1889+
self.check_set_op_does_not_crash(f)
1890+
1891+
def test_iteration_with_mutation(self):
1892+
def f1(a, b):
1893+
for x in a:
1894+
pass
1895+
for y in b:
1896+
pass
1897+
def f2(a, b):
1898+
for y in b:
1899+
pass
1900+
for x in a:
1901+
pass
1902+
def f3(a, b):
1903+
for x, y in zip(a, b):
1904+
pass
1905+
self.check_set_op_does_not_crash(f1)
1906+
self.check_set_op_does_not_crash(f2)
1907+
self.check_set_op_does_not_crash(f3)
1908+
1909+
1910+
class TestBinaryOpsMutating_Set_Set(TestBinaryOpsMutating, unittest.TestCase):
1911+
constructor1 = set
1912+
constructor2 = set
1913+
1914+
class TestBinaryOpsMutating_Subclass_Subclass(TestBinaryOpsMutating, unittest.TestCase):
1915+
constructor1 = SetSubclass
1916+
constructor2 = SetSubclass
1917+
1918+
class TestBinaryOpsMutating_Set_Subclass(TestBinaryOpsMutating, unittest.TestCase):
1919+
constructor1 = set
1920+
constructor2 = SetSubclass
1921+
1922+
class TestBinaryOpsMutating_Subclass_Set(TestBinaryOpsMutating, unittest.TestCase):
1923+
constructor1 = SetSubclass
1924+
constructor2 = set
1925+
1926+
1927+
class TestMethodsMutating(TestOperationsMutating):
1928+
1929+
def test_issubset_with_mutation(self):
1930+
self.check_set_op_does_not_crash(set.issubset)
1931+
1932+
def test_issuperset_with_mutation(self):
1933+
self.check_set_op_does_not_crash(set.issuperset)
1934+
1935+
def test_intersection_with_mutation(self):
1936+
self.check_set_op_does_not_crash(set.intersection)
1937+
1938+
def test_union_with_mutation(self):
1939+
self.check_set_op_does_not_crash(set.union)
1940+
1941+
def test_difference_with_mutation(self):
1942+
self.check_set_op_does_not_crash(set.difference)
1943+
1944+
def test_symmetric_difference_with_mutation(self):
1945+
self.check_set_op_does_not_crash(set.symmetric_difference)
1946+
1947+
def test_isdisjoint_with_mutation(self):
1948+
self.check_set_op_does_not_crash(set.isdisjoint)
1949+
1950+
def test_difference_update_with_mutation(self):
1951+
self.check_set_op_does_not_crash(set.difference_update)
1952+
1953+
def test_intersection_update_with_mutation(self):
1954+
self.check_set_op_does_not_crash(set.intersection_update)
1955+
1956+
def test_symmetric_difference_update_with_mutation(self):
1957+
self.check_set_op_does_not_crash(set.symmetric_difference_update)
1958+
1959+
def test_update_with_mutation(self):
1960+
self.check_set_op_does_not_crash(set.update)
1961+
1962+
1963+
class TestMethodsMutating_Set_Set(TestMethodsMutating, unittest.TestCase):
1964+
constructor1 = set
1965+
constructor2 = set
1966+
1967+
class TestMethodsMutating_Subclass_Subclass(TestMethodsMutating, unittest.TestCase):
1968+
constructor1 = SetSubclass
1969+
constructor2 = SetSubclass
1970+
1971+
class TestMethodsMutating_Set_Subclass(TestMethodsMutating, unittest.TestCase):
1972+
constructor1 = set
1973+
constructor2 = SetSubclass
1974+
1975+
class TestMethodsMutating_Subclass_Set(TestMethodsMutating, unittest.TestCase):
1976+
constructor1 = SetSubclass
1977+
constructor2 = set
1978+
1979+
class TestMethodsMutating_Set_Dict(TestMethodsMutating, unittest.TestCase):
1980+
constructor1 = set
1981+
constructor2 = dict.fromkeys
1982+
1983+
class TestMethodsMutating_Set_List(TestMethodsMutating, unittest.TestCase):
1984+
constructor1 = set
1985+
constructor2 = list
1986+
1987+
18021988
# Application tests (based on David Eppstein's graph recipes ====================================
18031989

18041990
def powerset(U):

‎Misc/NEWS.d/next/Core and Builtins/2022-02-04-04-33-18.bpo-46615.puArY9.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
When iterating over sets internally in ``setobject.c``, acquire strong references to the resulting items from the set. This prevents crashes in corner-cases of various set operations where the set gets mutated.

‎Objects/setobject.c

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,17 +1207,21 @@ set_intersection(PySetObject *so, PyObject *other)
12071207
while (set_next((PySetObject *)other, &pos, &entry)) {
12081208
key = entry->key;
12091209
hash = entry->hash;
1210+
Py_INCREF(key);
12101211
rv = set_contains_entry(so, key, hash);
12111212
if (rv < 0) {
12121213
Py_DECREF(result);
1214+
Py_DECREF(key);
12131215
return NULL;
12141216
}
12151217
if (rv) {
12161218
if (set_add_entry(result, key, hash)) {
12171219
Py_DECREF(result);
1220+
Py_DECREF(key);
12181221
return NULL;
12191222
}
12201223
}
1224+
Py_DECREF(key);
12211225
}
12221226
return (PyObject *)result;
12231227
}
@@ -1357,11 +1361,16 @@ set_isdisjoint(PySetObject *so, PyObject *other)
13571361
other = tmp;
13581362
}
13591363
while (set_next((PySetObject *)other, &pos, &entry)) {
1360-
rv = set_contains_entry(so, entry->key, entry->hash);
1361-
if (rv < 0)
1364+
PyObject *key = entry->key;
1365+
Py_INCREF(key);
1366+
rv = set_contains_entry(so, key, entry->hash);
1367+
Py_DECREF(key);
1368+
if (rv < 0) {
13621369
return NULL;
1363-
if (rv)
1370+
}
1371+
if (rv) {
13641372
Py_RETURN_FALSE;
1373+
}
13651374
}
13661375
Py_RETURN_TRUE;
13671376
}
@@ -1420,11 +1429,16 @@ set_difference_update_internal(PySetObject *so, PyObject *other)
14201429
Py_INCREF(other);
14211430
}
14221431

1423-
while (set_next((PySetObject *)other, &pos, &entry))
1424-
if (set_discard_entry(so, entry->key, entry->hash) < 0) {
1432+
while (set_next((PySetObject *)other, &pos, &entry)) {
1433+
PyObject *key = entry->key;
1434+
Py_INCREF(key);
1435+
if (set_discard_entry(so, key, entry->hash) < 0) {
14251436
Py_DECREF(other);
1437+
Py_DECREF(key);
14261438
return -1;
14271439
}
1440+
Py_DECREF(key);
1441+
}
14281442

14291443
Py_DECREF(other);
14301444
} else {
@@ -1515,17 +1529,21 @@ set_difference(PySetObject *so, PyObject *other)
15151529
while (set_next(so, &pos, &entry)) {
15161530
key = entry->key;
15171531
hash = entry->hash;
1532+
Py_INCREF(key);
15181533
rv = _PyDict_Contains(other, key, hash);
15191534
if (rv < 0) {
15201535
Py_DECREF(result);
1536+
Py_DECREF(key);
15211537
return NULL;
15221538
}
15231539
if (!rv) {
15241540
if (set_add_entry((PySetObject *)result, key, hash)) {
15251541
Py_DECREF(result);
1542+
Py_DECREF(key);
15261543
return NULL;
15271544
}
15281545
}
1546+
Py_DECREF(key);
15291547
}
15301548
return result;
15311549
}
@@ -1534,17 +1552,21 @@ set_difference(PySetObject *so, PyObject *other)
15341552
while (set_next(so, &pos, &entry)) {
15351553
key = entry->key;
15361554
hash = entry->hash;
1555+
Py_INCREF(key);
15371556
rv = set_contains_entry((PySetObject *)other, key, hash);
15381557
if (rv < 0) {
15391558
Py_DECREF(result);
1559+
Py_DECREF(key);
15401560
return NULL;
15411561
}
15421562
if (!rv) {
15431563
if (set_add_entry((PySetObject *)result, key, hash)) {
15441564
Py_DECREF(result);
1565+
Py_DECREF(key);
15451566
return NULL;
15461567
}
15471568
}
1569+
Py_DECREF(key);
15481570
}
15491571
return result;
15501572
}
@@ -1641,17 +1663,21 @@ set_symmetric_difference_update(PySetObject *so, PyObject *other)
16411663
while (set_next(otherset, &pos, &entry)) {
16421664
key = entry->key;
16431665
hash = entry->hash;
1666+
Py_INCREF(key);
16441667
rv = set_discard_entry(so, key, hash);
16451668
if (rv < 0) {
16461669
Py_DECREF(otherset);
1670+
Py_DECREF(key);
16471671
return NULL;
16481672
}
16491673
if (rv == DISCARD_NOTFOUND) {
16501674
if (set_add_entry(so, key, hash)) {
16511675
Py_DECREF(otherset);
1676+
Py_DECREF(key);
16521677
return NULL;
16531678
}
16541679
}
1680+
Py_DECREF(key);
16551681
}
16561682
Py_DECREF(otherset);
16571683
Py_RETURN_NONE;
@@ -1726,11 +1752,16 @@ set_issubset(PySetObject *so, PyObject *other)
17261752
Py_RETURN_FALSE;
17271753

17281754
while (set_next(so, &pos, &entry)) {
1729-
rv = set_contains_entry((PySetObject *)other, entry->key, entry->hash);
1730-
if (rv < 0)
1755+
PyObject *key = entry->key;
1756+
Py_INCREF(key);
1757+
rv = set_contains_entry((PySetObject *)other, key, entry->hash);
1758+
Py_DECREF(key);
1759+
if (rv < 0) {
17311760
return NULL;
1732-
if (!rv)
1761+
}
1762+
if (!rv) {
17331763
Py_RETURN_FALSE;
1764+
}
17341765
}
17351766
Py_RETURN_TRUE;
17361767
}

0 commit comments

Comments
 (0)