make sure coroutine locals are promptly destroyed

[richardsheridan]

This PR fixes a "bug" where unrolled_run hangs on to Cancelled tracebacks too long. They essentially persist until the next exception is thrown out of a task, which may be an indefinite period of time. This was not cleaned up with the earlier cyclic garbage bugfix #1770 because it is not cyclic garbage; it's actually a strong reference that lives unnecessarily long.

(Thinking out loud here) This is just the minimum possible fix. There are actually a number of locals akin to final_outcome in unrolled_run that may live up to _MAX_TIMEOUT seconds and contain references to tasks and potentially to hefty objects, but I haven't actually observed any issues along those lines yet. Scattering del statements everywhere is probably untenable, but factoring chunks of unrolled_run into function calls would "automate" that kind of cleanup, at the cost of some overhead.

[codecov]

Codecov Report

Merging #1864 (8170b42) into master (61ee58a) will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1864      +/-   ##
==========================================
+ Coverage   99.59%   99.64%   +0.04%     
==========================================
  Files         114      114              
  Lines       14549    14569      +20     
  Branches     1110     1110              
==========================================
+ Hits        14490    14517      +27     
+ Misses         42       36       -6     
+ Partials       17       16       -1     

Impacted Files	Coverage Δ
trio/_core/_run.py	100.00% <100.00%> (ø)
trio/_core/tests/test_run.py	100.00% <100.00%> (ø)
trio/testing/_check_streams.py	99.33% <0.00%> (+0.66%)	⬆️
trio/tests/test_socket.py	100.00% <0.00%> (+0.81%)	⬆️

[richardsheridan]

I totally meant to do a red-green test with these commits... yes...

[oremanj]

FWIW, I do think scattering del statements is entirely tenable. I'd be less excited about adding function call overhead on every task step.

[belm0]

worth a newsfragment?

[richardsheridan]

I think I got em all, but I would appreciate a double-check!

[belm0]

I think I got em all, but I would appreciate a double-check!

would it be hard to expand tests to cover the additional explicit deletes?

[richardsheridan]

Well I haven't been able to come up with a good test so far, let me stew on it another week. Also TIL except ... as exc blocks delete exc at the end of the block, so two of those are technically unnecessary!

[belm0]

Well I haven't been able to come up with a good test so far, let me stew on it another week. Also TIL except ... as exc blocks delete exc at the end of the block, so two of those are technically unnecessary!

So just keeping track, the deletions of msg, task, next_send, next_send_fn don't have test coverage? Consider adding a TODO in the code.

[belm0]

not sure how to trigger rebuild on sourcehut, will try close & re-open of PR