fix broken-channel bug in @trio.as_safe_channel
#3331
Conversation
Zac-HD
force-pushed
the
close-on-broken-resource
branch
3 times, most recently
from
b7a4513 to
ed7c6dc
Compare
Zac-HD
force-pushed
the
close-on-broken-resource
branch
from
ed7c6dc to
4407451
Compare
|
Just realized the "Encountered exception during cleanup of generator object ..." exception group... contains the exception group with multiple exceptions!? That looks weird when printed! I guess I should make a new issue about that. (unless that behavior makes perfect sense to you?) |
There was a problem hiding this comment.
I have a question and a comment about the tests, but this makes sense.
(I have the possibly strange opinion that we should one day have service nurseries, meaning that trio.as_safe_channel would be implemented s.t. the task calling the async generator should never raise... which actually runs counter to "I can close the channel" now that I think about it. Well, that's for another day.)
| yield # pragma: no cover | ||
|
|
||
| async with agen() as chan: | ||
| await chan.aclose() |
There was a problem hiding this comment.
Huh, it's a bit unintuitive to me that it's possible to not run any code within the async generator. But that can happen in sync generators too so I guess it's fine.
hmm, we could instead use |
Or async def test8() -> None:
@trio.as_safe_channel
async def agen():
await chan.aclose()
raise ValueError()
yield
async with agen() as chan:
async for _ in chan:
pass(I think. When I run that on Anyways, moved that to #3332. |
yeah, we should probably have an open issue for that in fact. |
|
Yeah turns out this PR doesn't fix that case so I've made #3333. I'm fine with this PR not fixing that. |
|
OK, merging this as an improvement on the status quo; I'd rather not block on a general fix to the cancellation problem. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3331 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 125 125
Lines 19547 19582 +35
Branches 1336 1341 +5
===============================================
+ Hits 19547 19582 +35
🚀 New features to boost your workflow:
|
We dropped this handler from #3197, presumably because the tests (...or production patterns) which trigger it are fairly esoteric. The
if not send_chan._state.open_receive_channels: breakclause is actually new, but lets us skip a pointless__anext__call in many cases which is a nice perf win.I just wish I'd noticed this in time for #3325, rather than when pulling it in the new version 🤦♂️