Avoid reference cycles when a sync() call fails

[spicysharkco]

When a coroutine run by sync() fails, the exception refers back to itself: its traceback holds the frame of _runner(), which holds result, which holds the exception, and the frame of sync() also keeps it in return_result. _run_coros_in_chunks, _copy, _cat and _cat_ranges do the same with first_exc/ex and their result lists.

So each failed call keeps its frames, and everything they refer to, until the cyclic garbage collector runs. For pipe_file that includes the data, and bytes don't count towards triggering a collection: 20 failed calls of 8 MiB each kept about 160 MiB with the collector disabled, and none with this change.

The fix clears those locals before the exception leaves the frame, as concurrent.futures.Future does. The new test checks that the exception of each failing call is freed by reference counting alone; it fails on master.

The same pattern is in generic.copy_file_op, SimpleCacheFileSystem.ls, WholeFileCacheFileSystem._cat_ranges and ReferenceFileSystem.cat. I can send those in a follow-up.

[martindurant]

Interesting - and thanks for looking into this.

I wonder, is this pattern known/documented elsewhere:

                try:
                    raise ex
                finally:
                    del ex, ...

This allows calling code to still catch the exception (and thereby keep the object alive for debugging). However, descending to the frame with pdb no longer allows inspecting the variable of interest - is there any solution to this?

It is a shame that we need to do this in multiple places, though.

General questions:
is the main problem with allowing cleanup via garbage collection the possibility of that cleanup happening on another thread? Or is it only about the retained data buffers you mention.

[martindurant]

Let's just include one method - they all operate the same way.

[martindurant]

Check: exc (maybe cleaned by ending the except-block).

[spicysharkco]

Thanks for the review. Rebased on master (changelog conflict only).

test_async.py:777: now one method, pipe, which goes through sync() and _run_coros_in_chunks. It still fails on master.

asyn.py:444: unchanged. The del doesn't touch exc, which the except block already clears. It removes first_exc, a separate name that outlives the block, and task, which holds the exception. Dropping either restores the cycle.

It is documented: the language reference says except E as N clears N because "with the traceback attached to them, they form a reference cycle with the stack frame, keeping all locals in that frame alive until the next garbage collection occurs". Plain assignments get no such cleanup. The stdlib does it by hand in Future.__get_result (concurrent/futures/_base.py: raise self._exception, then finally: self = None) and Condition.wait (asyncio/locks.py: err = None).

pdb still shows the exception, its traceback and every frame. Only the cleared names are missing (return_result, first_exc, task, and the result lists in _copy, _cat, _cat_ranges). The exception stays reachable as the caught one, or as $_exception in post-mortem pdb (3.13+; sys.exc_info()[1] before).

A helper can't clear the caller's locals, so cleanup must run in the raising frame. raise helper(out) (empty the list, return the exception) would only fit the three list-based sites, so I'd keep try/finally. The same cycle exists in generic.copy_file_op, SimpleCacheFileSystem.ls, WholeFileCacheFileSystem._cat_ranges and ReferenceFileSystem.cat; I can send those as a follow-up.

GC: mostly memory. Until a full collection, frames and buffers stay alive (20 failed 8 MiB calls held 160 MiB). Finalizers also run then, on whichever thread triggers it; in my test that was the IO thread.