gh-129013: Don't expose async_generator_wrapped_value in PY_YIELD callbacks

[ZeroIntensity]

• Issue: sys.monitoring PY_YIELD leaks internal async_generator_wrapped_value object #129013

[vstinner]

I don't know these parts of the Python code base, I cannot review your change.

[agronholm]

Thanks for this PR, I just found my way here as I was hit by the same bug!

[agronholm]

I had a couple of comments and a question: how is a user supposed to distinguish between yield and await in the callback?

[agronholm]

This would seem slightly cleaner.

Suggested change

	asyncio.run(main())
	sys.monitoring.set_events(0, 0)
	try:
	asyncio.run(main())
	finally:
	sys.monitoring.clear_tool_id(0)

[ZeroIntensity]

Not disagreeing, but I'd rather stay consistent with the rest of the file. try/finally doesn't seem to be used with set_events(0, 0) in these tests, so I'm going to skip it for now. Sounds reasonable to add that in a follow-up PR, though.

[ZeroIntensity]

how is a user supposed to distinguish between yield and await in the callback?

Hmm, I guess you could check if its an instance of asyncio.Future, assuming the user isn't yielding those for any other reason. Normal coroutines have this issue too; I guess it might be worth adding a PY_AWAIT event?

[agronholm]

how is a user supposed to distinguish between yield and await in the callback?

Hmm, I guess you could check if its an instance of asyncio.Future, assuming the user isn't yielding those for any other reason. Normal coroutines have this issue too; I guess it might be worth adding a PY_AWAIT event?

Futures being awaited are not the issue – it's things like asyncio.sleep(0) which just yield None. This would be indistinguishable from a plain yield. I'm just not sure how to fix this w/o breaking backwards compatibility.

[agronholm]

I do agree that ideally we should have separate PY_YIELD and PY_AWAIT.

[agronholm]

Perhaps the easiest backwards compatible solution is to expose the wrapper type and its contained value to Python code? @vstinner any thoughts so far?

[ZeroIntensity]

I would really rather not expose our implementation details to Python code :(

Do other implementations support sys.monitoring? Perhaps we could take inspiration from them.

[agronholm]

I would really rather not expose our implementation details to Python code :(

Do other implementations support sys.monitoring? Perhaps we could take inspiration from them.

sys.monitoring was added in 3.12 and both PyPy and GraalPy are still at 3.11, so no.