test and eliminate additional sources of cyclic garbage

[richardsheridan]

from MultiErrorCatcher.__exit__, _multierror.copy_tb, MultiError.filter, CancelScope.__exit__, and NurseryManager.__aexit__ methods. This was nearly impossible to catch until #1864 landed so it wasn't cleaned up in #1805 (or during the live stream: https://www.youtube.com/watch?v=E_jvJVYXUAk).

@belm0: curious if this shows up in your application benchmarking.

[codecov]

Codecov Report

Merging #2063 (e48d638) into master (2b33fe6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2063   +/-   ##
=======================================
  Coverage   99.56%   99.56%           
=======================================
  Files         114      114           
  Lines       14634    14688   +54     
  Branches     1119     1120    +1     
=======================================
+ Hits        14570    14624   +54     
  Misses         43       43           
  Partials       21       21           

Impacted Files	Coverage Δ
trio/_core/_multierror.py	98.91% <100.00%> (+0.01%)	⬆️
trio/_core/_run.py	100.00% <100.00%> (ø)
trio/_core/tests/test_multierror.py	99.30% <100.00%> (+0.03%)	⬆️
trio/_core/tests/test_run.py	100.00% <100.00%> (ø)

[belm0]

@belm0: curious if this shows up in your application benchmarking.

I haven't been keeping an eye on this lately, but I don't think it came up. Generally we try to avoid internal async API's that raise exceptions because multi-errors are so hard to deal with.

[richardsheridan]

Just noticed that CancelScope.__exit__ was mentioned as another possible source of cycles but it doesn't show up in the test yet. Happy to hear suggestions, or else I'll poke at it this weekend.

[richardsheridan]

I found a gross refcycle in the KI system. Here is the output of objgraph.show_backrefs:

Note that the args tuple is referenced both by the frame locals as well as the f_locals dict, leading to the spooky code at

trio/trio/_core/_ki.py

Lines 162 to 168 in f9d0e2a

	# don't create a refcycle if an exception is passed through and
	# re-raised, see test_cancel_scope_exit_doesnt_create_cyclic_garbage
	del args, kwargs
	# deep magic to remove refs via f_locals
	locals()
	# TODO: check if PEP558 changes the need for this call
	# https://github.com/python/cpython/pull/3640

an alternative solution might be to inline the locals()["@TRIO_KI_PROTECTION_ENABLED"]=True in CancelScope.__exit__, as that is a special case; that way we don't have to run extra bytecode every time KI protection is toggled.

[belm0]

an alternative solution might be to inline the locals()["@TRIO_KI_PROTECTION_ENABLED"]=True in CancelScope.exit, as that is a special case; that way we don't have to run extra bytecode every time KI protection is toggled.

this is appealing-- perhaps add another commit implementing it and let njs weigh in

[belm0]

LGTM. Add a newsfragment and check that PR description matches latest changes?