[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

STINNER Victor added the comment: New changeset 8d84adcd736619c2ce510dff1e7ffd3ab08df06a by Victor Stinner in branch 'master': bpo-32591: _PyErr_WarnUnawaitedCoroutine() sets source (GH-19247) https://github.com/python/cpython/commit/8d84adcd736619c2ce510dff1e7ffd3ab08df06a --

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by STINNER Victor : -- nosy: +vstinner nosy_count: 6.0 -> 7.0 pull_requests: +18605 pull_request: https://github.com/python/cpython/pull/19247 ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Thomas Wouters added the comment: New changeset 500a419a7a68c32650717b213f0f5ab0461bb16b by T. Wouters in branch '3.6': [3.6] bpo-32591: fix abort in _PyErr_WarnUnawaitedCoroutine during shutdown (GH-5337) (#6536)

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Yury Selivanov : -- stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Thomas Wouters : -- pull_requests: +6231 ___ Python tracker ___ ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Nathaniel Smith added the comment: New changeset 95e214713a7da4e5cbfb73f35b2119f76074fc3f by Nathaniel J. Smith in branch 'master': bpo-32591: silence deprecation warnings in test_coroutine (GH-5412)

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Nathaniel Smith : -- pull_requests: +5246 stage: resolved -> patch review ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Nathaniel Smith added the comment: Ah, gotcha. I'll take a look. (For posterity: the RuntimeWarnings Yury mentioned are also real, but they're from the branch Yury is testing for bpo-32605 / bpo-32703, nothing to do with this issue.) --

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: Specifically "DeprecationWarning: get_coroutine_wrapper is deprecated" needs to be silenced as part of this issue. -- ___ Python tracker

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: Nathaniel, test_coroutines raises a bunch of Runtime and Deprecation warnings after this PR. Could you please make a PR to silence them? /Users/yury/dev/pydev/cpython/Lib/test/test_coroutines.py:1032: RuntimeWarning: coroutine

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: > OK. I hope both of you keep an eye on this and actively try to test and break it! That's the plan! -- ___ Python tracker

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Guido van Rossum added the comment: OK. I hope both of you keep an eye on this and actively try to test and break it! -- ___ Python tracker

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Yury Selivanov : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: Nathaniel's latest PR looks good and fixes the crash. FWIW losing some warnings during interpreter shutdown isn't exactly a new problem, therefore I think there's nothing wrong with the new API here. For instance, I've seen asyncio's

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: New changeset dba976b8a28d6e5daa66ef31a6a7c694a9193f6a by Yury Selivanov (Nathaniel J. Smith) in branch 'master': bpo-32591: fix abort in _PyErr_WarnUnawaitedCoroutine during shutdown (#5337)

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Guido van Rossum added the comment: Maybe it's better to roll this back and take a deep breath. The feature freeze should not be treated as an excuse to cram in things at the last minute. Before this week I had never heard of this proposal. And the sarcasm is a bad start of

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Nathaniel Smith : -- pull_requests: +5184 stage: resolved -> patch review ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Nathaniel Smith added the comment: Right, I *knew* I should be nervous about calling into Python from a C-level destructor... what's happening is: - Somehow, that coroutine object is managing to survive until the very, very end of the shutdown sequence (PyImport_Cleanup). I'm

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Yury Selivanov : -- resolution: fixed -> status: closed -> open ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: The following snippet crashes with SIGABRT: import asyncio async def f(): pass asyncio.gather(f()) llvm tells that the crash is in _PyGen_Finalize/_PyErr_WarnUnawaitedCoroutine --

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: New changeset 3510334361d6d39aad89e4e0d9bccd0695668583 by Yury Selivanov in branch 'master': bpo-32591: Fix PyExc_WarnFormat call (follow-up commit) (#5263)

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Yury Selivanov : -- pull_requests: +5109 ___ Python tracker ___ ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: Merged! Thank you, Nathaniel! -- resolution: -> fixed stage: patch review -> resolved status: open -> closed type: -> enhancement versions: -Python 3.8 ___ Python tracker

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Guido van Rossum added the comment: Yes, I'm OK with this plan. Note that I've only read Nathaniel's original message and Yury's last message, and I've not reviewed the PR, but getting rid of that eyesore sounds good, and it seems you two have reached agreement. --

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: Guido, I'd like to go forward with this and merge Nathaniel's PR. Quick summary: * This issue adds new APIs: a sys.set_coroutine_origin_tracking_depth(depth) function and a 'cr_origin' property to native coroutine objects. * By using

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Change by Nathaniel Smith : -- keywords: +patch pull_requests: +5097 stage: -> patch review ___ Python tracker ___

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: > I think the simplest thing is probably to write the warning code in Python > and stash it in Lib/_corohelper.py or something. I'd put the helper that accepts a list of traceback-like tuples and emits a warning in warnings.py and a C

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Nathaniel Smith added the comment: > I'm not sure how exactly the warning print logic will work with a list of > tuples in cr_origin, I propose to start working on the implementation and > figure that out during the review process. I think the simplest thing is probably to

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

Yury Selivanov added the comment: +1 on on the API and for adding it in 3.7 and for deprecating set_coroutine_wrapepr. This will simplify asyncio code and other event loops (like uvloop) and will lead to less overhead on asyncio debug mode. I always disliked the

[issue32591] Deprecate sys.set_coroutine_wrapper and replace it with more focused API(s)

New submission from Nathaniel Smith : sys.set_coroutine_wrapper is a problematic API. It was added to solve a specific problem: asyncio debug mode wanting to track where coroutine objects are created, so that when unawaited coroutines are GC'ed, the warning they print can