Skip to content

deprecate the asyncio child watchers system #94597

Description

@graingert

Deprecate the child watchers system and the policy system in favour of
asyncio.Runner(loop_factory=asyncio.ProactorEventLoop/asyncio.SelectorEventLoop/uvloop.new_event_loop)
that would be deprecating:

asyncio.get_event_loop() # already deprecated unless the loop is running 
asyncio.set_event_loop()  # asyncio.set_event_loop(None) should probably be exempt
asyncio.get_event_loop_policy()
asyncio.set_event_loop_policy()  # asyncio.set_event_loop_policy(None) should probably be exempt
asyncio.set_child_watcher()
asyncio.get_child_watcher()  # `_make_subprocess_transport` will instead attempt to use os.open_pidfd and fallback to starting a thread

I'd also like to introduce a new API: asyncio.EventLoop implemented as:

if sys.platform == "win32":
    EventLoop = ProactorEventLoop
else:
    EventLoop = SelectorEventLoop

asyncio.new_event_loop() will issue a DeprecationWarning if the current policy is not the default policy, and then in 3 releases become an alias of asyncio.EventLoop

Originally posted by @graingert in #93896 (comment)

Activity

  1. graingert commented on Jul 6, 2022

    @graingert
    ContributorAuthor

    Some background to this proposed deprecation:

    This is a more comprehensive version of #82772

    @serhiy-storchaka proposed deprecating set_event_loop() in #93453 (comment)

    But maybe we should first deprecate set_event_loop()? It will be a no-op now.

    @asvetlov noted that
    get_event_loop_policy().get_event_loop() was not deprecated by oversight in #83710 (comment)

    IMHO, asyncio.set_event_loop() and policy.get_event_loop()/policy.set_event_loop() are not deprecated by oversight.

  2. gvanrossum commented on Jul 6, 2022

    @gvanrossum
    Member

    We are in dire need of more asyncio experts. @1st1 this isn't urgent but would be nice to have your perspective in time to do this in 3.12.

  3. kumaraditya303 commented on Jul 6, 2022

    @kumaraditya303
    Contributor

    For 3.12 IMO we should deprecate MultiLoopWatcher #82504 and others which have race condition and other issue. Once that's done we may deprecate the entire child watcher system but just removing MultiLoopWatcher would be a good start.

  4. graingert commented on Jul 7, 2022

    @graingert
    ContributorAuthor
  5. gvanrossum commented on Oct 6, 2022

    @gvanrossum
    Member

    We discussed this at the sprint and we agree that there are many things wrong with the child watchers and the policy system.

    Deprecating child watchers: @1st1 thinks these should be done per loop (like uvloop does), not globally by the policy. Much more discussion on this topic is already in #82772. Bottom line, we agree to deprecate it, details remain to be seen.

    Deprecating policies: Yes please. The policies no longer serve a real purpose. Loops are always per thread, there is no need to have a "current loop" when no loop is currently running. The only thing we still need is a loop factory, so perhaps instead of an API for getting/setting a global "policy", we could have an API for getting/setting a global "loop factory".

    I'm fine with the EventLoop alias (it ties up a loose end), but I recommend that the API for creating a new event loop (when not using runners) should be asyncio.new_event_loop(), not asyncio.EventLoop().

    We should totally deprecate set_event_loop() (even with None argument). At that point we can make get_event_loop() an alias for get_running_loop() (or the other way around -- I prefer calling get_event_loop() :-).

  6. graingert commented on Oct 7, 2022

    @graingert
    ContributorAuthor

    so perhaps instead of an API for getting/setting a global "policy", we could have an API for getting/setting a global "loop factory".

    I disagree, that's the most painful part of the policy system that I'm looking to deprecate here in favor of passing an explicit loop_factory to asyncio.Runner. The behavior of Runner should be to pick the best event loop by default, if people need to change the behavior of Runner they should pass an explicit factory, if they really need to patch this behavior globally for the whole process they can use a monkey patch

  7. gvanrossum commented on Oct 7, 2022

    @gvanrossum
    Member

    that's the most painful part of the policy system

    What exactly is most painful? That there's a global default for something? To me the painful thing is that the policy system is over-engineered, you have to create a class that overrides new_event_loop, instantiate it, and call set_event_loop_policy with the instance. That is just classic Java. You shouldn't need to have to create a class, just a function.

    I totally agree that most people should use run() or Runner, but I disagree that we should deprecate all other workflows. To me, Runner is just a convenience class.

  8. gvanrossum commented on Oct 7, 2022

    @gvanrossum
    Member

    I discussed this with Yury and he convinced me that we don't need a global loop factory. Instead we should just have a loop_factory=None keyword arg to asyncio.run().

    We still have to come up with a way to transition to a world where child watching is per-loop instead of global though.

  9. gvanrossum commented on Oct 7, 2022

    @gvanrossum
    Member

    Good news. @1st1 has a simple refactoring of PidfdChildWatcher that makes it independent from the main loop -- just like ThreadedChildWatcher. Once that is merged (PR is forthcoming) we can also merge @kumaraditya303's PR GH-98024, and then we can start deprecating all other child watcher implementations.

    We can then also deprecate set_child_watcher() (both the asyncio function and the policy method) and eventually we can move the child watcher out of the policy. There's some hand-waving here because in theory people could subclass the default policy class and override get_child_watcher() to construct their own child watcher -- we'll have to deprecate that too somehow.

    But all this builds a road to a world where policies are no longer needed and eventually no longer exist.

  10. 26 remaining items

  11. graingert commented on Oct 18, 2022

    @graingert
    ContributorAuthor

    FWIW it seems that Jupyter has a legitimate reason to override the default policy (with another one of the predefined ones), see #93453 (comment).

    The WindowsSelectorEventLoopPolicy shouldn't be needed with tornado 6.2 where it runs a selector in a background thread so the main event loop can be the ProactorEventLoop

  12. added a commit that references this issue on Nov 12, 2022
  13. graingert commented on Oct 11, 2023

    @graingert
    ContributorAuthor

    We are in dire need of more asyncio experts. @1st1 this isn't urgent but would be nice to have your perspective in time to do this in 3.12.

    It seems @1st1 is in favor of deprecating the rest of the policy system: https://twitter.com/1st1/status/1711007413275365590?t=PGAYW5447_2JZdgiIsu6eQ&s=19 can we do this for 3.13?

  14. gvanrossum commented on Oct 11, 2023

    @gvanrossum
    Member

    I am fine with that. But I am not someone with a lot of time for it. In 3.13 all we need is some deprecations. Can you help with that?

  15. added a commit that references this issue on Oct 11, 2023
  16. graingert commented on Oct 11, 2023

    @graingert
    ContributorAuthor

    @gvanrossum I can definitely help adding deprecations!

  17. added a commit that references this issue on Oct 12, 2023
  18. graingert commented on Oct 12, 2023

    @graingert
    ContributorAuthor

    I want to cleanup the calls to

    def teardownModule():
        asyncio.set_event_loop_policy(None)

    this is now possible for tests that just use asyncio.run but it's not yet possible for IsolatedAsyncioTestCase I have a design here https://discuss.python.org/t/support-setting-the-loop-factory-in-isolatedasynciotestcase/36027/1

  19. kaithar commented on Jan 28, 2024

    @kaithar

    There's also an intriguing custom watcher in chaperone but this package appears unmaintained (last commits in 2016). It appears a modified clone of FastChildWatcher.

    I attempted to bring Chaperone up to 3.11 last year, the reason it uses a custom watcher and the reason I was looking at them again is that anyone wanting to install a SIGCHLD handler under asyncio is going to stumble onto the conflict it produces with the child watchers.

    Something designed to run as pid 1 should really be doing zombie reaping for unknown processes and a custom watcher seems to be the only reliable way to install a default behaviour. Prior to 3.8 the default was SafeChildWatcher, which has a conflict, so a custom was required. ThreadedChildWatcher from 3.8 and PidfdChildWatcher from 3.9 seem like they won't conflict, however only the former offers a tracking mechanism via ThreadedChildWatcher._threads[pid] to allow determining if asyncio is going to call os.waitpid for that process. PidfdChildWatcher has no callback lookup so there's nothing to hijack. Double calls to waitpid are likely to be unpleasant.

    The proper method of dealing with this (admittedly niche) use case still seems to be a custom child watcher or not using asyncio at all. If I'm interpreting the comments correctly, this will now require an entire policy instead of being able to just override the watcher?

  20. kumaraditya303 commented on Jun 23, 2024

    @kumaraditya303
    Contributor

    This is complete, the deprecation of policy system can be done in separate issue.

  21. moved this from Todo to Done in asyncioon Jun 23, 2024
  22. changed the title [-]deprecate the asyncio child watchers system and policy system[/-] [+]deprecate the asyncio child watchers system[/+] on Jun 23, 2024
  23. added a commit that references this issue on Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

3.12only security fixesstdlibStandard Library Python modules in the Lib/ directorytopic-asyncio

Projects

  • Status
    Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions