diff --git a/Include/internal/pycore_immutability.h b/Include/internal/pycore_immutability.h index a68cd24124b063d..b204680f84ecd47 100644 --- a/Include/internal/pycore_immutability.h +++ b/Include/internal/pycore_immutability.h @@ -21,6 +21,13 @@ PyAPI_FUNC(int) _PyTracingRegion_DetachIgnoreRegionRefs(PyObject* region); PyAPI_FUNC(int) _PyTracingRegion_Attach(PyObject* region, _PyCown_owner_id_t owner); PyAPI_FUNC(int) _PyTracingRegion_AttachIgnoreRegionRefs(PyObject* region); +#ifdef Py_DEBUG +/* Tests only: makes every close pause for `ns` nanoseconds between its trace + * and the validation, with the thread detached, so that other threads can + * interfere with it. 0 disables the pause. */ +PyAPI_FUNC(void) _PyTracingRegion_SetTestPause(PyTime_t ns); +#endif + /* Returns the region's metadata node, allocating it if this is the first * region reference the current close has found. Borrowed, and only valid while * the region stays closed. The caller must hold `_PyWeakref_Lock`. */ diff --git a/Lib/test/test_freeze/test_tracing_region_ported.py b/Lib/test/test_freeze/test_tracing_region_ported.py new file mode 100644 index 000000000000000..6269cea27b4d84e --- /dev/null +++ b/Lib/test/test_freeze/test_tracing_region_ported.py @@ -0,0 +1,991 @@ +"""Tests ported from a different implementation of regions (the "experiment" +branch, Lib/test/test_free_threading/test_region.py and test_cown.py). + +Only the tests of what closing a region means were ported, not those of the +experiment's machinery (colours, claims, grace periods, tags). Where the +experiment expects something that this implementation does differently by +design, the test keeps the experiment's expectation and is marked with +`diverges()`, which records why. +""" + +import copy +import gc +import pickle +import random +import threading +import time +import unittest +import weakref + +from immutable import TracingRegion as Region, Cown +from test import support +from test.support import import_helper, threading_helper + +_testinternalcapi = import_helper.import_module("_testinternalcapi") +region_close = _testinternalcapi.region_close +region_is_closed = _testinternalcapi.region_is_closed + + +def close(r): + """Close `r` without the cown's check for references to the bridge. + Return True if `r` is now closed, False if it could not be closed.""" + try: + region_close(r) + except RuntimeError: + return False + return True + + +def region_state(r): + return "closed" if region_is_closed(r) else "open" + + +def diverges(reason): + """The experiment expects what the test checks, this implementation does + something else by design. The test is an expected failure, so that it + reports an unexpected success should the behaviour change.""" + def decorator(test): + test.diverges = reason + return unittest.expectedFailure(test) + return decorator + + +needs_close_pause = unittest.skipUnless( + hasattr(_testinternalcapi, "region_set_close_pause"), + "needs the close pause of debug builds") + + +class close_pause: + """While inside, every close pauses for `seconds` between its trace and + the validation, with the thread detached, so that other threads can + interfere with it.""" + + def __init__(self, seconds): + self.seconds = seconds + + def __enter__(self): + _testinternalcapi.region_set_close_pause(self.seconds) + return self + + def __exit__(self, *exc): + _testinternalcapi.region_set_close_pause(0) + + +def run_in_thread(func, *args): + """Run func(*args) in a new thread; return its result or raise its + exception.""" + result = [] + + def run(): + try: + result.append((True, func(*args))) + except BaseException as e: + result.append((False, e)) + + t = threading.Thread(target=run) + t.start() + t.join() + ok, value = result[0] + if not ok: + raise value + return value + + +class Foo: + pass + + +class OwnRef(weakref.ref): + # A weak reference of our own: the basic one may be shared. + __slots__ = () + + +def make_graph(): + # A graph of mortal objects created by the calling thread, with cycles + # and edges to objects that are not part of regions (immortals, types). + # Values are computed at run time, a constant would be shared with the + # code object. + foo = Foo() + foo.items = [Foo(), {"key": Foo(), "n": pow(10, 30)}, + (Foo(), str(10**6))] + foo.items[0].back = foo + foo.cls = Foo + foo.none = None + s = {str(10**7), 1} + return [foo, s, foo.items] + + +def nest(*objs): + """Return a new region whose contents is a list of `objs`.""" + r = Region() + r.contents = list(objs) + return r + + +class TestRegionStates(unittest.TestCase): + + def test_new_region_is_open(self): + self.assertEqual(region_state(Region()), "open") + + def test_contents(self): + r = Region() + foo = Foo() + r.contents = foo + self.assertIs(r.contents, foo) + del r.contents + with self.assertRaises(AttributeError): + r.contents + + def test_close(self): + r = Region() + r.contents = [Foo(), {"a": Foo()}] + self.assertIs(close(r), True) + self.assertEqual(region_state(r), "closed") + # Closing a closed region is a no-op. + self.assertIs(close(r), True) + self.assertEqual(region_state(r), "closed") + + def test_access_opens(self): + for access in (lambda r: r.contents, + lambda r: setattr(r, "contents", Foo()), + lambda r: delattr(r, "contents"), + lambda r: r.__dict__, + lambda r: vars(r), + lambda r: object.__getattribute__(r, "__dict__")): + with self.subTest(access=access): + r = Region() + r.contents = Foo() + self.assertTrue(close(r)) + access(r) + self.assertEqual(region_state(r), "open") + + def test_repr_does_not_open(self): + r = Region() + close(r) + self.assertIn("closed", repr(r)) + self.assertEqual(region_state(r), "closed") + + +class TestRegionType(unittest.TestCase): + + def test_not_subclassable(self): + with self.assertRaises(TypeError): + class Sub(Region): + pass + + def test_no_pickle_or_copy(self): + r = Region() + for proto in range(pickle.HIGHEST_PROTOCOL + 1): + with self.subTest(proto=proto): + with self.assertRaises(TypeError): + pickle.dumps(r, proto) + with self.assertRaises(TypeError): + copy.copy(r) + with self.assertRaises(TypeError): + copy.deepcopy(r) + + def test_constructor_takes_no_arguments(self): + with self.assertRaises(TypeError): + Region(1) + with self.assertRaises(TypeError): + Region(contents=1) + + def test_gc(self): + # A region in a reference cycle is collected. + r = Region() + foo = Foo() + foo.region = r + r.contents = foo + wr = weakref.ref(foo) + del r, foo + gc.collect() + self.assertIsNone(wr()) + + +class TestIsolation(unittest.TestCase): + + def check_closes(self, contents): + r = Region() + r.contents = contents + del contents + self.assertIs(close(r), True) + self.assertEqual(region_state(r), "closed") + return r + + def check_does_not_close(self, r): + self.assertIs(close(r), False) + self.assertEqual(region_state(r), "open") + + def test_empty(self): + self.check_closes(None) + + def test_isolated_graph(self): + self.check_closes(make_graph()) + + def test_leaf(self): + self.check_closes(str(10**8)) + + def test_strings_are_not_part_of_regions(self): + # Strings are immutable: shared with the outside or between regions, + # they do not keep a region from closing. + shared = str(10**9) + self.check_closes([shared, Foo()]) + r1 = self.check_closes([shared]) + r2 = self.check_closes({shared: 1}) + self.assertEqual(region_state(r1), "closed") + self.assertEqual(region_state(r2), "closed") + + def test_index_by_string_closes(self): + # dict_traverse() does not report the keys of all-string dicts, so + # strings referenced both as keys and from elsewhere in the graph can + # look externally referenced. + class Item: + def __init__(self, i): + self.name = f"item{i}" + def build(): + items = [Item(i) for i in range(5)] + return [items, {o.name: o for o in items}] + self.check_closes(build()) + + def test_str_subclass_is_part_of_regions(self): + class S(str): + pass + r = Region() + r.contents = [S("mutable attributes")] + inner = r.contents[0] + self.check_does_not_close(r) + del inner + self.assertIs(close(r), True) + + def test_reference_back_to_region(self): + # References to the region itself are allowed, also from inside. + r = Region() + graph = make_graph() + graph[0].region = r + r.contents = graph + del graph + others = [r, r] + self.assertIs(close(r), True) + + def test_external_reference_to_interior(self): + r = Region() + graph = make_graph() + r.contents = graph + inner = graph[0].items[1] + del graph + self.check_does_not_close(r) + del inner + self.assertIs(close(r), True) + + def test_external_reference_to_contents(self): + r = Region() + graph = make_graph() + r.contents = graph + self.check_does_not_close(r) + del graph + self.assertIs(close(r), True) + + def test_reference_from_other_container(self): + r = Region() + graph = make_graph() + r.contents = graph + outside = {"alias": graph[1]} + del graph + self.check_does_not_close(r) + outside.clear() + self.assertIs(close(r), True) + + @threading_helper.requires_working_threading() + def test_reference_held_by_other_thread(self): + r = Region() + r.contents = make_graph() + holding = threading.Event() + release = threading.Event() + + def hold(obj): + holding.set() + release.wait() + + t = threading.Thread(target=hold, args=(r.contents[0].items[0],)) + t.start() + holding.wait() + try: + self.check_does_not_close(r) + finally: + release.set() + t.join() + self.assertIs(close(r), True) + + def test_object_in_two_regions(self): + shared = Foo() + r1, r2 = Region(), Region() + r1.contents = [shared] + r2.contents = [shared] + del shared + self.check_does_not_close(r1) + self.check_does_not_close(r2) + + def test_reopen_and_close_again(self): + r = self.check_closes(make_graph()) + alias = r.contents # opens the region + self.assertEqual(region_state(r), "open") + self.check_does_not_close(r) + del alias + self.assertIs(close(r), True) + + +class TestSubregions(unittest.TestCase): + # The builders below return only the outermost region, so that the test + # holds no references into it. + + def test_open_subregion_closes_with_parent(self): + outer = nest(nest(Foo()), Foo()) + self.assertTrue(close(outer)) + inner = outer.contents[0] # opens the outer region only + self.assertEqual(region_state(outer), "open") + self.assertEqual(region_state(inner), "closed") + + def test_three_levels(self): + outer = nest(nest(nest(Foo()), Foo())) + self.assertTrue(close(outer)) + middle = outer.contents[0] + inner = middle.contents[0] + self.assertEqual(region_state(inner), "closed") + # Closing the outer region again walks the open middle one and + # counts the closed inner one. + del middle, inner + self.assertTrue(close(outer)) + + def test_subregion_with_external_reference(self): + for inner_closed in (False, True): + with self.subTest(inner_closed=inner_closed): + inner = nest(Foo()) + if inner_closed: + self.assertTrue(close(inner)) + outer = nest(inner) + self.assertFalse(close(outer)) + self.assertEqual(region_state(outer), "open") + + @diverges("a failed close leaves the subregions it closed on the way " + "closed, the experiment restores them to open") + def test_subregion_with_external_reference_keeps_its_state(self): + inner = nest(Foo()) + outer = nest(inner) + self.assertFalse(close(outer)) + self.assertEqual(region_state(inner), "open") + + def test_failure_leaves_closed_subregions_closed(self): + x = Foo() + closed = nest(Foo()) + self.assertTrue(close(closed)) + outer = nest(nest(Foo()), closed, x) # x is referenced from here + del closed + self.assertFalse(close(outer)) + _, closed_inner, _ = outer.contents + self.assertEqual(region_state(closed_inner), "closed") + del closed_inner, _ + del x + self.assertTrue(close(outer)) + + @diverges("a failed close leaves the subregions it closed on the way " + "closed, the experiment restores them to open") + def test_failure_restores_subregions(self): + x = Foo() + outer = nest(nest(Foo()), x) # x is referenced from here + self.assertFalse(close(outer)) + open_inner = outer.contents[0] + self.assertEqual(region_state(open_inner), "open") + + def test_references_to_subregion(self): + # A parent may refer to a subregion any number of times, and a + # region's graph may refer back to the region. + def build(): + inner = Region() + inner.contents = [inner, {"self": inner}] + return nest(inner, inner, (inner,)) + outer = build() + self.assertTrue(close(outer)) + # The closed subregion's back-references are recorded. + outer.contents + self.assertTrue(close(outer)) + + def test_edges_across_regions(self): + def into_subregion(): + x = Foo() + return nest(nest(x), x) + def into_parent(): + x = Foo() + outer = Region() + inner = nest(x) + outer.contents = [x, inner] + return outer + def to_parent_region(): + outer = Region() + outer.contents = [nest(outer)] + return outer + def to_sibling(): + second = nest(Foo()) + return nest(nest(second), second) + def to_grandparent_object(): + x = Foo() + return nest(x, nest(nest(x))) + for build in (into_subregion, into_parent, to_parent_region, + to_sibling, to_grandparent_object): + with self.subTest(build=build.__name__): + outer = build() + self.assertFalse(close(outer)) + self.assertEqual(region_state(outer), "open") + outer.contents = None # break any cycle + + @threading_helper.requires_working_threading() + def test_subregion_of_other_thread(self): + # The experiment refuses this while the other thread lives, since + # there an open region is owned by the thread that opened it. Here + # regions have no owning thread, so the subregion is closed with its + # parent, after its objects were made shared. + made = threading.Event() + done = threading.Event() + box = [] + def make(): + box.append(nest(Foo())) # created by this thread, open + made.set() + done.wait() + t = threading.Thread(target=make) + t.start() + try: + made.wait() + outer = nest(box.pop()) + self.assertTrue(close(outer)) + finally: + done.set() + t.join() + inner = outer.contents[0] + self.assertEqual(region_state(inner), "closed") + + def test_cown_is_not_walked(self): + # A cown's value is its own business, even if it is not isolated (so + # that the cown could not be released), and references to a cown + # from outside are fine. + x = Foo() + c = Cown(nest(x)) + r = nest(c) + self.assertTrue(close(r)) + c.value = None + c.release() + + def test_weakref_into_subregion(self): + def build(): + obj = Foo() + return nest(nest(obj)), OwnRef(obj) + outer, wr = build() + self.assertTrue(close(outer)) + inner = outer.contents[0] # opens the outer region only + self.assertIsNone(wr()) # the referent's region is closed + + @diverges("closing clears weak references from outside for good, the " + "experiment refuses them only while the region is closed") + def test_weakref_into_subregion_after_opening(self): + def build(): + obj = Foo() + return nest(nest(obj)), OwnRef(obj) + outer, wr = build() + self.assertTrue(close(outer)) + outer.contents[0].contents # opens both + self.assertIsInstance(wr(), Foo) + + def test_empty_subregions(self): + self.assertTrue(close(nest(nest(Foo()), nest()))) + + +class TestRegionWeakrefs(unittest.TestCase): + + def region_with(self, obj): + r = Region() + r.contents = [obj] + return r + + @threading_helper.requires_working_threading() + def test_external_weakref_refused_after_close(self): + obj = Foo() + wr = weakref.ref(obj) + r = self.region_with(obj) + del obj + self.assertTrue(close(r)) + self.assertIsNone(wr()) + self.assertIsNone(run_in_thread(wr)) + + @diverges("closing clears weak references from outside for good, the " + "experiment refuses them only while the region is closed") + def test_external_weakref_usable_after_opening(self): + obj = Foo() + wr = weakref.ref(obj) + r = self.region_with(obj) + del obj + self.assertTrue(close(r)) + contents = r.contents + self.assertIs(wr(), contents[0]) + + def test_proxy_refused_after_close(self): + obj = Foo() + obj.x = 1 + proxy = weakref.proxy(obj) + r = self.region_with(obj) + del obj + self.assertTrue(close(r)) + with self.assertRaises(ReferenceError): + proxy.x + + @diverges("closing clears weak references from outside for good, the " + "experiment refuses them only while the region is closed") + def test_proxy_usable_after_opening(self): + obj = Foo() + obj.x = 1 + proxy = weakref.proxy(obj) + r = self.region_with(obj) + del obj + self.assertTrue(close(r)) + r.contents + self.assertEqual(proxy.x, 1) + + def test_weak_edges_inside_region(self): + obj = Foo() + inner = weakref.ref(obj) + r = Region() + r.contents = [obj, inner] + del obj, inner + self.assertTrue(close(r)) + contents = r.contents + self.assertIs(contents[1](), contents[0]) + + def test_outgoing_weakref_rejected(self): + outside = Foo() + r = Region() + r.contents = [weakref.ref(outside)] + self.assertIs(close(r), False) + self.assertEqual(region_state(r), "open") + + def test_outgoing_weakref_to_excluded_or_dead_allowed(self): + # A weak reference to a type (not part of regions) is allowed, and + # so is a dead one. + r = Region() + r.contents = [OwnRef(Foo)] + self.assertTrue(close(r)) + dead = Foo() + r = Region() + r.contents = [OwnRef(dead)] + del dead + self.assertTrue(close(r)) + + def test_weak_value_dictionary(self): + obj = Foo() + d = weakref.WeakValueDictionary() + d["k"] = obj + r = self.region_with(obj) + del obj + self.assertTrue(close(r)) + self.assertNotIn("k", d) + + +@threading_helper.requires_working_threading() +class TestConcurrentClose(unittest.TestCase): + + @needs_close_pause + def test_access_while_closing_fails_the_close(self): + # The experiment refuses the access instead. Here the access goes + # through and the close fails. + r = Region() + r.contents = make_graph() + accessed = [] + + def access(): + time.sleep(0.1) # the close is pausing + accessed.append(r.contents is not None) + + t = threading.Thread(target=access) + with close_pause(0.4): + t.start() + self.assertIs(close(r), False) + t.join() + self.assertEqual(accessed, [True]) + self.assertEqual(region_state(r), "open") + self.assertIs(close(r), True) + + def check_transient_access(self, keep): + # While the close pauses, another thread takes a reference to the + # contents without going through the region's attributes + # (gc.get_referents() is an escape hatch) and, unless `keep`, drops + # it again. The count is then back to what the trace recorded; only + # the trace flag tells. + r = Region() + r.contents = make_graph() + kept = [] + + def touch(): + time.sleep(0.1) # the close is pausing + if keep: + kept.append(gc.get_referents(r)) + else: + gc.get_referents(r) + + t = threading.Thread(target=touch) + with close_pause(0.4): + t.start() + self.assertIs(close(r), False) + t.join() + self.assertEqual(region_state(r), "open") + kept.clear() + self.assertIs(close(r), True) + + @needs_close_pause + @unittest.skipUnless(support.Py_GIL_DISABLED, + "with the GIL a trace is never interleaved") + def test_reference_taken_and_dropped_during_close(self): + self.check_transient_access(keep=False) + + @needs_close_pause + @unittest.skipUnless(support.Py_GIL_DISABLED, + "with the GIL a trace is never interleaved") + def test_reference_taken_during_close(self): + self.check_transient_access(keep=True) + + def test_overlapping_regions_closed_concurrently(self): + # Each thread creates and closes its own region; all regions contain + # the same object. + nregions = 4 + shared = [Foo()] + results = [] + regions = [] # keeps every region alive until all are closed + barrier = threading.Barrier(nregions) + + def close_own(): + r = Region() + r.contents = [shared[0], Foo()] + regions.append(r) + barrier.wait() # all regions exist + del shared[:] + barrier.wait() + results.append(close(r)) + + threads = [threading.Thread(target=close_own) for _ in range(nregions)] + with threading_helper.start_threads(threads): + pass + self.assertEqual(results, [False] * nregions) + + def test_disjoint_regions_closed_concurrently(self): + nregions = 8 + results = [] + barrier = threading.Barrier(nregions) + + def close_own(): + r = Region() + r.contents = make_graph() + barrier.wait() + results.append(close(r)) + + threads = [threading.Thread(target=close_own) for _ in range(nregions)] + with threading_helper.start_threads(threads): + pass + self.assertEqual(results, [True] * nregions) + + def test_gc_during_close(self): + stop = threading.Event() + + def collect(): + # Pause between collections: back-to-back stop-the-world pauses + # would starve the other threads. + while not stop.is_set(): + gc.collect() + time.sleep(0.001) + + t = threading.Thread(target=collect) + t.start() + try: + for _ in range(50): + r = Region() + r.contents = make_graph() + self.assertIs(close(r), True) + finally: + stop.set() + t.join() + + def test_stress(self): + # Threads keep taking and dropping references to interior objects + # (through the escape hatch) while the main thread keeps closing and + # reopening the region. Results may be either; nothing may crash or + # leak. + r = Region() + r.contents = make_graph() + wr = weakref.ref(r.contents[0]) + stop = threading.Event() + errors = [] + + def churn(): + try: + while not stop.is_set(): + for obj in gc.get_referents(r): + for inner in gc.get_referents(obj): + pass + except BaseException as e: + errors.append(e) + + threads = [threading.Thread(target=churn) for _ in range(4)] + results = set() + with threading_helper.start_threads(threads): + try: + deadline = time.monotonic() + 1.0 + while time.monotonic() < deadline: + results.add(close(r)) + alias = r.contents # reopen + del alias + finally: + stop.set() + self.assertEqual(errors, []) + self.assertLessEqual(results, {True, False}) + self.assertIs(close(r), True) + del r + gc.collect() + self.assertIsNone(wr()) + + +class acquired: + """`with acquired(c):` acquires the cown and releases it at the end. The + experiment's cowns are context managers themselves.""" + + def __init__(self, cown): + self.cown = cown + + def __enter__(self): + self.cown.acquire() + return self.cown + + def __exit__(self, *exc): + self.cown.release() + + +class TestCownRegions(unittest.TestCase): + # Releasing a cown closes the region it holds, and requires the cown's + # `value` to be the only reference to the region. Cowns are acquired when + # created. + + def test_release_closes_region(self): + c = Cown(nest(Foo(), [Foo()])) + self.assertEqual(region_state(c.value), "open") + c.release() + with acquired(c): + self.assertEqual(region_state(c.value), "closed") + + def test_release_closes_subregions(self): + c = Cown(nest(nest(Foo()))) + c.release() + with acquired(c): + inner = c.value.contents[0] + self.assertEqual(region_state(inner), "closed") + del inner + + def test_reference_to_region(self): + r = nest(Foo()) + c = Cown(r) + with self.assertRaisesRegex(RuntimeError, "references to the bridge"): + c.release() + self.assertIs(c.value, r) # still acquired + del r + c.release() + + def test_region_not_isolated(self): + x = Foo() + c = Cown(nest(x)) + with self.assertRaisesRegex(RuntimeError, "could not be closed"): + c.release() + self.assertEqual(region_state(c.value), "open") + del x + c.release() + + def test_back_references(self): + r = Region() + r.contents = [r, {"r": r}] + c = Cown(r) + del r + c.release() + c.acquire() + c.release() # closed: recorded back-references + with acquired(c): + c.value.contents # opened: walked again + + def test_move_between_cowns(self): + a = Cown(nest(Foo())) + a.release() + b = Cown() + a.acquire() + b.value = a.value + a.value = None + b.release() + a.release() + with acquired(b): + self.assertEqual(region_state(b.value), "closed") + + def test_same_region_in_two_cowns(self): + a = Cown(nest(Foo())) + b = Cown() + b.value = a.value + with self.assertRaisesRegex(RuntimeError, "references to the bridge"): + b.release() + with self.assertRaisesRegex(RuntimeError, "references to the bridge"): + a.release() + b.value = None + b.release() + a.release() + + def test_cown_in_region(self): + inner = Cown() + inner.release() + c = Cown(nest(inner, Foo())) + c.release() + with acquired(inner): # shared with the region: fine + pass + + @threading_helper.requires_working_threading() + def test_handover(self): + c = Cown(nest()) + c.release() + def append(): + with acquired(c): + c.value.contents.append(Foo()) + for _ in range(3): + run_in_thread(append) + with acquired(c): + self.assertEqual(len(c.value.contents), 3) + + +# Helpers for the stress test. Each works in its own frame, so that its +# local references into the region are gone before the cown is released. + +def bump(c, rng): + # The region holds [counter, [objects...], subregion]; the subregion + # holds [counter]. + data = c.value.contents + data[0] += 1 + objs = data[1] + if objs and rng.random() < 0.5: + objs.pop() + else: + f = Foo() + f.me = f # a cycle: only the GC frees it once popped + objs.append(f) + if rng.random() < 0.3: + inner = data[2].contents # opens the subregion + inner[0] += 1 + return 2 + return 1 + + +def total(c): + data = c.value.contents + return data[0] + data[2].contents[0] + + +def stress_region(): + inner = Region() + inner.contents = [0] + r = Region() + r.contents = [0, [], inner] + return r + + +@threading_helper.requires_working_threading() +class TestCownStress(unittest.TestCase): + + def test_pass_regions_around(self): + self.pass_regions_around() + + def test_pass_regions_around_with_gc(self): + # As above, while another thread keeps collecting garbage. + stop = threading.Event() + def collect(): + while not stop.is_set(): + gc.collect() + time.sleep(0.002) + t = threading.Thread(target=collect) + t.start() + try: + self.pass_regions_around() + finally: + stop.set() + t.join() + + def pass_regions_around(self): + # Threads acquire cowns at random, update the regions in them (and + # their subregions), sometimes swap the regions of two cowns (without + # opening them), and release; every release closes what was opened. + NTHREADS = 6 + NCOWNS = 4 + ITERATIONS = 300 + cowns = [Cown(stress_region()) for _ in range(NCOWNS)] + for c in cowns: + c.release() + bumps = [0] * NTHREADS + errors = [] + + def release(c): + # A failed release leaves the cown acquired; empty it and + # release it, so that the other workers do not wait for it + # forever (the test fails on the recorded error). + try: + c.release() + except Exception as e: + errors.append(e) + c.value = None + c.release() + + def worker(i): + rng = random.Random(i) + try: + for _ in range(ITERATIONS): + if rng.random() < 0.2: + a, b = sorted(rng.sample(range(NCOWNS), 2)) + cowns[a].acquire() + cowns[b].acquire() + try: + cowns[a].value, cowns[b].value = ( + cowns[b].value, cowns[a].value) + finally: + release(cowns[b]) + release(cowns[a]) + else: + c = cowns[rng.randrange(NCOWNS)] + c.acquire() + try: + if c.value is not None: + bumps[i] += bump(c, rng) + finally: + release(c) + except BaseException as e: + errors.append(e) + + threads = [threading.Thread(target=worker, args=(i,)) + for i in range(NTHREADS)] + with threading_helper.start_threads(threads): + pass + self.assertEqual(errors, []) + found = 0 + for c in cowns: + with acquired(c): + self.assertEqual(region_state(c.value), "closed") + found += total(c) + self.assertEqual(found, sum(bumps)) + + def test_release_closed_regions(self): + # Acquiring and releasing without opening the region only reads its + # reference count; many threads doing so do not interfere. + c = Cown(stress_region()) + c.release() + def worker(): + for _ in range(2000): + with acquired(c): + pass + threading_helper.run_concurrently(worker, nthreads=8) + with acquired(c): + self.assertEqual(region_state(c.value), "closed") + + +if __name__ == "__main__": + unittest.main() diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index c2647d405e25bcb..5817b543539f04f 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -24,6 +24,7 @@ #include "pycore_function.h" // _PyFunction_GET_BUILTINS #include "pycore_gc.h" // PyGC_Head #include "pycore_hashtable.h" // _Py_hashtable_new() +#include "pycore_immutability.h" // _PyTracingRegion_Close() #include "pycore_import.h" // _PyImport_ClearExtension() #include "pycore_initconfig.h" // _Py_GetConfigsAsDict() #include "pycore_instruction_sequence.h" // _PyInstructionSequence_New() @@ -35,6 +36,7 @@ #include "pycore_pylifecycle.h" // _PyInterpreterConfig_InitFromDict() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_runtime_structs.h" // _PY_NSMALLPOSINTS +#include "pycore_time.h" // _PyTime_FromSecondsObject() #include "pycore_unicodeobject.h" // _PyUnicode_TransformDecimalAndSpaceToASCII() #include "clinic/_testinternalcapi.c.h" @@ -2418,7 +2420,59 @@ set_vectorcall_nop(PyObject *self, PyObject *func) Py_RETURN_NONE; } +static int +check_region(PyObject *obj) +{ + if (!Py_IS_TYPE(obj, &_PyTracingRegion_Type)) { + PyErr_Format(PyExc_TypeError, "expected a TracingRegion, not '%.200s'", + Py_TYPE(obj)->tp_name); + return -1; + } + return 0; +} + +/* Closes a region, without the cown's check that it holds the only reference + * to the bridge. Raises if the region could not be closed. */ +static PyObject * +region_close(PyObject *self, PyObject *region) +{ + if (check_region(region) < 0) { + return NULL; + } + if (_PyTracingRegion_Close(region) < 0) { + return NULL; + } + Py_RETURN_NONE; +} + +static PyObject * +region_is_closed(PyObject *self, PyObject *region) +{ + if (check_region(region) < 0) { + return NULL; + } + return PyBool_FromLong(_PyTracingRegion_IsClosed(region)); +} + +#ifdef Py_DEBUG +static PyObject * +region_set_close_pause(PyObject *self, PyObject *seconds) +{ + PyTime_t ns; + if (_PyTime_FromSecondsObject(&ns, seconds, _PyTime_ROUND_CEILING) < 0) { + return NULL; + } + _PyTracingRegion_SetTestPause(ns); + Py_RETURN_NONE; +} +#endif + static PyMethodDef module_functions[] = { + {"region_close", region_close, METH_O}, + {"region_is_closed", region_is_closed, METH_O}, +#ifdef Py_DEBUG + {"region_set_close_pause", region_set_close_pause, METH_O}, +#endif {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, {"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS}, diff --git a/Objects/cownobject.c b/Objects/cownobject.c index f253a3da3f70065..13e6a3cc6dd55c7 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -268,6 +268,10 @@ static int PyCown_reachable(_PyCownObject *self, visitproc visit, void *arg) { } static int PyCown_clear(_PyCownObject *self) { + // The GC may have cleared the cown already, before deallocating it. + if (self->value == NULL) { + return 0; + } if (_PyImmutability_CanViewAsDeepImmutable(self->value)) { Py_CLEAR(self->value); return 0; diff --git a/Objects/tracingregionobject.c b/Objects/tracingregionobject.c index 113232531438830..7d9988b3e3be216 100644 --- a/Objects/tracingregionobject.c +++ b/Objects/tracingregionobject.c @@ -8,7 +8,7 @@ #include "pycore_weakref.h" #include "pycore_cown.h" #include "pycore_regionref.h" -#ifdef Py_GIL_DISABLED +#if defined(Py_GIL_DISABLED) || defined(Py_DEBUG) #include "pycore_parking_lot.h" // _PyParkingLot_Park() #include "pycore_time.h" // _PyDeadline_Init() #endif @@ -1732,6 +1732,32 @@ static int _validate_region_closed(TracingRegionObject *region_obj, region_trace return 0; } +#ifdef Py_DEBUG +static PyTime_t region_test_pause_ns = 0; + +void _PyTracingRegion_SetTestPause(PyTime_t ns) { + _Py_atomic_store_int64_relaxed(®ion_test_pause_ns, ns); +} + +/* Sleeps for the pause set by `_PyTracingRegion_SetTestPause()`. The thread is + * detached meanwhile, which also releases the GIL. */ +static void region_test_pause(void) { + PyTime_t ns = _Py_atomic_load_int64_relaxed(®ion_test_pause_ns); + if (ns <= 0) { + return; + } + PyTime_t deadline = _PyDeadline_Init(ns); + // Stack address to park on; nobody unparks it. + uint8_t sleeper = 0; + uint8_t expected = 0; + PyTime_t remaining; + while ((remaining = _PyDeadline_Get(deadline)) > 0) { + (void)_PyParkingLot_Park(&sleeper, &expected, sizeof(sleeper), + remaining, NULL, /*detach=*/1); + } +} +#endif + static int _try_close_region(PyObject *region_obj, tree_trace_state_t *tree_trace_state) { assert(Region_Check(region_obj)); TracingRegionObject* region = _PyTRegion_CAST(region_obj); @@ -1850,6 +1876,10 @@ static int _try_close_region(PyObject *region_obj, tree_trace_state_t *tree_trac goto error; } +#ifdef Py_DEBUG + region_test_pause(); +#endif + SUCCEEDS(_validate_region_closed(region, &state)); SUCCEEDS(_region_close(region, state.bridge_rc, state.visited, state.has_weak_refs)); @@ -1857,6 +1887,11 @@ static int _try_close_region(PyObject *region_obj, tree_trace_state_t *tree_trac goto finally; error: region_trace_res = TRACE_RES_ERR; +#ifdef _Py_PYRONA_INTERPRETER_SHARING + // The trace may have moved objects into the region's GC list already. + // They must go back to the local GC, since the region remains open. + gc_list_dissolve(®ion->gc_list); +#endif region_clear_tracing_mark(region); finally: Py_CLEAR(item); @@ -2271,7 +2306,8 @@ int _PyTracingRegion_Close(PyObject* op) { #ifdef _Py_PYRONA_INTERPRETER_SHARING TracingRegionObject *self = _PyTRegion_CAST(op); - assert(gc_list_is_empty(&self->gc_list)); + // A closed region keeps its members in its GC list + assert(!region_is_open(self) || gc_list_is_empty(&self->gc_list)); #endif return try_close_region_tree(op); diff --git a/report.md b/report.md new file mode 100644 index 000000000000000..b6690c061c88346 --- /dev/null +++ b/report.md @@ -0,0 +1,336 @@ +# Porting the experiment's region tests to tracing regions + +2026-10-09. Base commit: `0a6ec0a3d3` (Detect concurrent accesses via the bridge). + +## Summary + +The tests that check what closing a region means were ported from the other +implementation of regions in `~/w/python/experiment`. That suite is +`Lib/test/test_free_threading/test_region.py` and `test_cown.py`, about 200 +tests. The ported tests are in +`Lib/test/test_freeze/test_tracing_region_ported.py`: 62 tests in all, +including 5 that check deliberate differences between the two designs. + +The port found 6 bugs, all in closing regions and releasing cowns: + +| # | Bug | Builds | Status | +|---|-----|--------|--------| +| 1 | A failed close leaves objects in the region's private GC list | GIL | **fixed** | +| 2 | Wrong assertion in `_PyTracingRegion_Close` crashes debug builds | GIL debug | **fixed** | +| 3 | `PyCown_clear` dereferences NULL when the GC has cleared the cown already | free-threaded | **fixed** | +| 4 | `_Py_CheckTracingFlag` never clears the flag, so concurrent accesses go undetected | free-threaded | open | +| 5 | A parent cannot close an open subregion whose bridge is owned by another thread | free-threaded | open | +| 6 | Closing waits on owners that have stopped servicing their merge queue, until the 1 s timeout | free-threaded | open | + +With the fixes, every ported test passes on a GIL debug build. On a +free-threaded debug build, 4 ported tests fail because of the open bugs 4–6. + +## What was ported + +The experiment closes regions differently: it uses colours, claims, FREE +objects, grace periods and weak reference tags. Most of its tests check that +machinery and mean nothing here. Only tests that check behaviour, meaning which +regions can close and what happens around a close, were ported. + +| Ported class | Tests | Source | +|---|---|---| +| `TestRegionStates` | 5 | open/close states; attribute access opens a region, `repr` does not | +| `TestRegionType` | 4 | not subclassable, not picklable or copyable, constructor arguments, collected in cycles | +| `TestIsolation` | 13 | the core rule: no references from outside into the region | +| `TestSubregions` | 13 | nesting, edges between regions, subregions with outside references, cowns inside regions | +| `TestRegionWeakrefs` | 8 | weak references into, out of, and inside a region | +| `TestConcurrentClose` | 7 | closes interleaved with other threads, the GC, and churn through `gc.get_referents` | +| `TestCownRegions` | 9 | releasing a cown closes its region, references to the bridge, moving regions between cowns | +| `TestCownStress` | 3 | threads handing regions around through cowns, with and without a concurrent GC | + +These were not ported: + +- **Implementation-specific:** `TestColours`, `TestClaims*`, `TestFreeObjects*`, + `TestUntracking`, `TestRetracking`, `TestOverrides`, `TestTableCache`, + `TestTagEpochs`, `TestRegionHandle`, `TestDecrementDuringValidation` and + `TestDereferenceDuringTagging`. +- **Different concepts:** + - `TestOwnership`: the experiment's open regions belong to a thread. + - `TestWeakrefGate`: the experiment tags weak references instead of clearing them. + - `TestExternalReferences`: the experiment's `region_close` returns a reference count. + - Tests that force a region into a state through `region_set_state`. +- **Grace-period tests,** and tests that need the experiment's + `spin_without_safe_point` helper. +- **The cown API itself** (`TestCown`, `TestCownThreads`). The two cown APIs + differ: the experiment's cowns start out released and are context managers. + +### API mapping + +| experiment | here | +|---|---| +| `region_close(r, timeout)` | `_testinternalcapi.region_close(r)` (new) | +| `region_state(r)` | `_testinternalcapi.region_is_closed(r)` (new) | +| `check_window` / `region_set_check_pauses` | `_testinternalcapi.region_set_close_pause(s)` (new, debug builds) | +| `Cown()` starts released; `with c:` | `Cown(v)` starts acquired; the test uses an `acquired(c)` helper | +| a region has one `contents` slot | a region has any attributes; the tests use `contents` | + +### Differences between the designs + +Where the experiment expects something this implementation does differently on +purpose, the test keeps the experiment's expectation and is marked +`@diverges(reason)`. That makes it an expected failure, and an unexpected pass +is reported. All 5 fail as expected: + +- **Subregions after a failed close.** A failed close leaves the subregions it + closed along the way closed. The experiment restores them to open. + - `test_subregion_with_external_reference_keeps_its_state` + - `test_failure_restores_subregions` +- **Weak references from outside.** Closing clears them for good. The + experiment only refuses them while the region is closed, so they work again + once it is opened. + - `test_weakref_into_subregion_after_opening` + - `test_external_weakref_usable_after_opening` + - `test_proxy_usable_after_opening` + +`test_subregion_of_other_thread` was adapted rather than marked. The experiment +refuses the close while the thread that opened the subregion is alive. Here +regions have no owning thread, so the test expects the close to succeed (see +bug 5). + +## Test hooks added + +In `Modules/_testinternalcapi.c`: + +- `region_close(r)` calls `_PyTracingRegion_Close()`. It raises `RuntimeError` + if the region cannot be closed. Unlike `Cown.release()`, it does not check for + references to the bridge. +- `region_is_closed(r)` calls `_PyTracingRegion_IsClosed()`. +- `region_set_close_pause(seconds)` exists in debug builds only. It makes every + close pause after its trace and before the validation and commit. The thread + is detached during the pause, which also releases the GIL. This lets a test + interfere with a close at the point where it is vulnerable. + +The pause is implemented as `_PyTracingRegion_SetTestPause()` in +`Objects/tracingregionobject.c` and declared in +`Include/internal/pycore_immutability.h`. All of it is under `#ifdef Py_DEBUG`. + +## Results + +Both builds were configured with `--with-pydebug`, one with `--disable-gil` and +one without. + +| | GIL debug | free-threaded debug | +|---|---|---| +| `test_tracing_region_ported` | pass (5 expected failures, 2 skipped) | 4 failures (bugs 4–6) | +| `test_tracing_region` (existing) | pass | intermittent timeout in `test_close_objects_owned_by_blocked_thread` (bug 6) | +| `test_freeze`, `test_weakref`, `test_sys`, `test_capi`, `test_free_threading` | pass (1,971 tests) | pass apart from the failures above | + +The free-threaded failures: + +| Test | Bug | +|---|---| +| `TestConcurrentClose.test_reference_taken_and_dropped_during_close` | 4 | +| `TestSubregions.test_subregion_of_other_thread` | 5 or 6, depending on timing | +| `TestCownStress.test_pass_regions_around` | 5 and 6 | +| `TestCownStress.test_pass_regions_around_with_gc` | 5 and 6 | + +To run the ported tests: + +``` +./python -m test test_freeze.test_tracing_region_ported -v +``` + +## Fixed bugs + +### 1. A failed close leaves objects in the region's private GC list + +`Objects/tracingregionobject.c`, in `_try_close_region()`, GIL builds +(`_Py_PYRONA_INTERPRETER_SHARING`). + +A trace moves each object it visits into `region->gc_list`. Two failure paths +merge the list back into the interpreter's GC: the restart path and the +`external_rc > 0` path. The shared `error:` label did not. So any other error +left the region open with objects in its private list, hidden from the GC. Such +errors include an unmovable object, the parent-cycle error, a concurrent access +(`TRACING_FAILED`), and an allocation failure. + +The next close then hit `assert(gc_list_is_empty(...))`, and so did the +region's finalizer. On a first attempt the bug is usually masked: freezing a +string sets `state->gc_list = NULL` before any objects are moved. On a retry, +the list is in use from the start. + +It was found by `TestSubregions.test_edges_across_regions` (the `to_parent_region` +case) and `TestConcurrentClose.test_access_while_closing_fails_the_close`. + +**Fix:** dissolve the list on the `error:` path as well. Dissolving a list that +is already empty does nothing. + +### 2. Wrong assertion in `_PyTracingRegion_Close` + +`_PyTracingRegion_Close()` asserted that `self->gc_list` is empty. A closed +region keeps its members in that list, so closing an already-closed region +tripped the assertion. That happens in the existing +`TestRegionOpening.test_release_closed_region`. The result was that the +existing suite crashed on every GIL debug build. Release builds compile the +assertion out. + +**Fix:** `assert(!region_is_open(self) || gc_list_is_empty(&self->gc_list))`. + +### 3. `PyCown_clear` dereferences NULL + +`Objects/cownobject.c`, free-threaded builds, where cowns are tracked by the GC. + +When a cown with an immutable value is part of cyclic garbage, the GC calls its +`tp_clear`, and `Py_CLEAR(self->value)` sets the value to NULL. When the cown +is deallocated afterwards, `cown_dealloc_owned()` calls `PyCown_clear()` again, +which passes NULL to `_PyImmutability_CanViewAsDeepImmutable()`. Reproducer: + +```python +import gc +from immutable import Cown +c = Cown() +c.release() +l = [c] +l.append(l) +del c, l +gc.collect() # segfault on free-threaded builds +``` + +**Fix:** return early from `PyCown_clear()` when `self->value` is NULL. + +## Open bugs + +### 4. `_Py_CheckTracingFlag` never clears the flag + +`Python/pystate.c`: + +```c +void _Py_CheckTracingFlag(PyObject *ob) { + if ((_Py_OB_FLAGS_LOAD(ob) & _Py_REGION_TRACE_FLAG) == 0) { + _Py_OB_FLAG_REMOVE(ob, _Py_REGION_TRACE_FLAG); + } +} +``` + +This clears the flag only when it is already clear, so it never does anything. +As a result, the first check in `_validate_region_closed_visit()` cannot fail. +Suppose another thread takes a reference to an object in the region and drops +it again while the region is being traced. The reference count is then back to +the value the trace recorded, and the close succeeds. A reference that is kept +is still caught, by the reference count check. + +The commit that added this function is titled "Let's benchmark RC's new slow +branch", so the condition may have been inverted on purpose to measure the cost +of the branch. If not, the condition should be `!= 0`. + +**Test:** `TestConcurrentClose.test_reference_taken_and_dropped_during_close`. +It pauses the close and has another thread call `gc.get_referents(r)`, which +increments and decrements the region's `__dict__`. The close is expected to +fail but succeeds. + +### 5. An open subregion whose bridge is owned by another thread cannot close with its parent + +`try_close_region_tree()` and `_trace_visit_bridge_ref()` in +`Objects/tracingregionobject.c`. A parent containing such a subregion is traced +as follows: + +1. **Attempt 1.** The parent's trace finds the subregion open. It queues the + subregion and restarts. The open path never reaches `_move_obj()`, so the + ownership of the subregion's bridge is not checked. +2. **The subregion closes.** Its own trace may need two attempts if its objects + are non-local too. +3. **Attempt 2.** The parent's trace now finds the subregion closed and moves + its bridge with `_move_obj()`. The bridge is owned by another thread, so the + bridge is queued for a reference count merge and the trace restarts again. +4. **Attempt 3** would be needed, but `PER_REGION_TRACE_LIMIT` is 2, so the + close fails with "the region … could not be closed after 2 tracing attempts". + +Reproducer (deterministic): + +```python +import threading, _testinternalcapi as t +from immutable import TracingRegion as Region +box, made, done = [], threading.Event(), threading.Event() +def make(): + r = Region() # the bridge is owned by this thread + r.contents = [1] + box.append(r) + made.set() + done.wait() +th = threading.Thread(target=make); th.start(); made.wait() +outer = Region() +outer.contents = [box.pop()] +try: + t.region_close(outer) # RuntimeError: ... could not be closed after 2 tracing attempts +finally: + done.set(); th.join() +``` + +Instrumenting the restarts during `test_pass_regions_around` shows this +sequence for every region that fails with this error. In each case the +subregion's bridge was created by the main thread and the parent was released +by a worker. + +**Possible fixes:** + +- Check the ownership of the bridge, and queue it, when `_trace_visit_bridge_ref()` + first finds the subregion open. The second attempt then has nothing new to + find. +- Don't count restarts that were caused only by non-local objects against the + limit. Those restarts end once the objects are merged. + +### 6. Closing waits on owners that have stopped servicing their merge queue + +`_move_obj()` and `tree_trace_state_wait_non_local_objects()` in +`Objects/tracingregionobject.c`, and `_Py_brc_queue_object()` in `Python/brc.c`. + +When a trace finds an object owned by another thread, it calls +`_Py_brc_queue_object()`. Then, before retrying, it waits up to +`NON_LOCAL_MERGE_TIMEOUT_MS` (1 s) for the owner to merge the object's +reference count. + +`_Py_brc_queue_object()` has a fast path for owners that are detached at the +time of the call: it suspends the owner and merges the object itself. If the +owner is attached instead, the object goes onto the owner's queue, and the +owner merges it at its next eval breaker. If the owner blocks before then, it +is detached but never services its queue. Nobody merges the object, and the +closing thread times out with "timed out waiting for N non-local object(s) to +become shared". + +Logging the stuck objects during `test_pass_regions_around` shows this pattern: + +- the stuck objects are `Foo` instances with an unmerged reference count + (`ob_ref_shared == 8`); +- their owners are detached the whole time; +- in the cases examined, the owner was blocked in `Cown.acquire()` on the cown + that the closing thread was trying to release. + +So the closer waits for the owner to merge, and the owner waits for the closer +to release the cown. Only the timeout ends the wait. The same race explains why +the existing `test_close_objects_owned_by_blocked_thread` sometimes times out: +the owner can still be attached when its objects are queued, just before it +blocks in `release_owner.wait()`. + +Two explanations were tested and ruled out: + +- **The cown lock keeps waiting threads attached.** It does not: `cown_lock()` + wraps the blocking `_PyMutex_LockTimed()` in `Py_BEGIN_ALLOW_THREADS`. Changing + it to `_PY_LOCK_DETACH` made no difference. +- **A waiting closer does not service its own merge queue.** Calling + `_Py_brc_merge_refcounts()` in each iteration of the wait loop did not remove + the timeouts. + +**Possible fix:** in each iteration of the wait loop, find the owners of the +objects that are still pending. Suspend each one that is detached, as the fast +path in `_Py_brc_queue_object()` does, and merge its pending objects on its +behalf. The owner's queue would have to be cleared of those objects too, or +merging an object twice must be harmless. + +## Changes to the repository + +| File | Change | +|---|---| +| `Lib/test/test_freeze/test_tracing_region_ported.py` | new: the ported tests | +| `Modules/_testinternalcapi.c` | the test hooks `region_close`, `region_is_closed`, `region_set_close_pause` | +| `Include/internal/pycore_immutability.h` | declares `_PyTracingRegion_SetTestPause()` (debug builds) | +| `Objects/tracingregionobject.c` | the test pause (debug builds); fixes for bugs 1 and 2 | +| `Objects/cownobject.c` | fix for bug 3 | +| `report.md` | this report | + +Nothing has been committed. The in-tree build (`--enable-optimizations`, GIL) +has not been rebuilt with these changes.