Deprecate Process Child Watchers

@vstinner

BPO	38591
Nosy	@vstinner, @benjaminp, @asvetlov, @1st1, @aeros

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/aeros'
closed_at = None
created_at = <Date 2019-10-25.19:23:29.645>
labels = ['3.9', 'expert-asyncio']
title = 'Deprecate Process Child Watchers'
updated_at = <Date 2019-11-15.13:21:36.776>
user = 'https://github.com/aeros'

bugs.python.org fields:

activity = <Date 2019-11-15.13:21:36.776>
actor = 'aeros'
assignee = 'aeros'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2019-10-25.19:23:29.645>
creator = 'aeros'
dependencies = []
files = []
hgrepos = []
issue_num = 38591
keywords = []
message_count = 22.0
messages = ['355372', '355373', '355375', '355376', '355379', '355380', '355381', '355390', '355394', '355395', '355396', '355409', '355419', '355421', '356002', '356586', '356594', '356596', '356599', '356639', '356669', '356672']
nosy_count = 5.0
nosy_names = ['vstinner', 'benjamin.peterson', 'asvetlov', 'yselivanov', 'aeros']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue38591'
versions = ['Python 3.9']

Proposal:
Deprecate the alternative process watcher implementation to ThreadedChildWatcher, MultiLoopChildWatcher.

Motivation:
The idea for this proposal came from a comment from Andrew Svetlov in #60756:
"I believe after polishing ThreadedChildWatcher we can start deprecation of watchers subsystem at all, it generates more problems than solves."

Although Andrew suggested the idea of deprecating the process watchers subsystem entirely, I think that it would be adequate to start with just deprecating MultiLoopChildWatcher and proceeding from there. This is because the other three watcher implementations are significantly more stable in comparison (based on number of issues), have less complex implementations, and have far more widespread usage across public repositories. I would not be opposed to deprecating the other alternative watchers as well, but MultiLoopChildWatcher likely has the most justification for removal.

Details:
The class MultiLoopChildWatcher has a fairly complex implementation, causes a number of issues, is fairly unstable, is rarely utilized in comparison with the rest of the watchers API. It seems to have become a significant burden on maintenance for asyncio development without providing a significant benefit to the vast majority of users.

Current unresolved MultiLoopChildWatcher issues:
https://bugs.python.org/issue38323
https://bugs.python.org/issue38182
https://bugs.python.org/issue37573

(3 out of the 5 open process watcher issues are caused by MultiLoopChildWatcher)

GitHub code usage comparison:
MultiLoopChildWatcher: https://github.com/search?l=Python&q=MultiLoopChildWatcher&type=Code (20 results)
ThreadedChildWatcher: https://github.com/search?l=Python&q=ThreadedChildWatcher&type=Code (77 results)
FastChildWatcher: https://github.com/search?l=Python&q=FastChildWatcher&type=Code (4,426 results)
SafeChildWatcher: https://github.com/search?l=Python&q=SafeChildWatcher&type=Code (7,007 results)
All of asyncio usage: https://github.com/search?l=Python&q=asyncio&type=Code (599,131 results)

Note that for the above results, it also includes matches in the CPython repository and for repositories that have exact copies of Lib/asyncio/unix_events.py. Also, ThreadedChildWatcher likely has significantly less results since it's already the default watcher returned by asyncio.get_child_watcher(), so there's less need to explicitly declare it compared to the others. There are of course private repositories and non-GitHub repositories this doesn't include, but it should provide a general idea of overall usage.

My experience in interacting with asyncio users is similar to the results. The process watchers subsystem seems to be minimally used compared to the rest of asyncio, with ThreadedChildWatcher likely being the most used as the default returned from asyncio.get_child_watcher(). I have not personally seen a realistic example of usage of MultiLoopChildWatcher outside of python/cpython, and all of the results returned on GitHub were copies of Lib/asyncio/unix_events.py or Lib/test/test_asyncio/test_subprocess.py.

I would be interested in working on this issue if it is approved, as I think it would provide a significant long term reduction in maintenance for asyncio; allowing us to focus on the improvement and development of other features that benefit a far larger audience.

Didn't we just add MultiLoopChildWatcher in 3.8?

Didn't we just add MultiLoopChildWatcher in 3.8?

Yep, that's correct: https://docs.python.org/3/library/asyncio-policy.html#asyncio.MultiLoopChildWatcher

I wasn't aware of that, but it would explain the current lack of usage. I'm generally in favor of Andrew's idea to deprecate the watchers subsystem, but perhaps if we go through with that we should remove MultiLoopChildWatcher from 3.8 (if it's not too late to do so) instead of deprecating it.

but perhaps if we go through with that we should remove MultiLoopChildWatcher from 3.8 (if it's not too late to do so) instead of deprecating it.

I'll leave that up to Andrew to decide, but I'd be +1 to drop it asap, especially if we want to eventually deprecate watchers.

Speaking of watchers -- big +1 from me to drop them all at some point. I would start as early as 3.9.

Linux has pidfd now, freebsd/macos has kqueue, windows has its own apis for watching processes. Threads can be the backup method for OSes that lack proper APIs for watching multiple processes (without using SIGCHLD etc).

Speaking of watchers -- big +1 from me to drop them all at some point. I would start as early as 3.9.

Yeah that was my initial plan, to start the deprecation in 3.9 and finalize the removal in 3.11. We might be able to make an exception for MultiLoopChildWatcher and just remove it from 3.8 though.

I think it will be necessary to fix the issues in the near future if we want to keep MultiLoopChildWatcher in 3.8 (as they apply to 3.8 and 3.9). I would certainly not be ecstatic about the idea of removing something that was just recently added (similar to how we had to revert the new streaming changes due to API design), but I'm even less in favor of keeping something around that's not stable in a final release.

Based on my investigation of https://bugs.python.org/issue38323, it seems like it will be a rather complex issue to solve.

Kyle, why are you resetting the status to "Pending"?

I think it will be necessary to fix the issues in the near future if we want to keep MultiLoopChildWatcher in 3.8 (as they apply to 3.8 and 3.9). I would certainly not be ecstatic about the idea of removing something that was just recently added (similar to how we had to revert the new streaming changes due to API design), but I'm even less in favor of keeping something around that's not stable in a final release.

Your opinion on this is duly noted.

It would be great to hear what Andrew and Victor think about this.

Kyle, why are you resetting the status to "Pending"?

That was an accident, I think I had that set already on the page and submitted my comment just after you did yours.

I'm going to change the title of the issue to "Deprecate Process Child Watchers". I started the proposal bit more conservatively because I thought that it might be easier to just deprecate MultiLoopChildWatcher at first. But seeing as it was just added in 3.8, I don't think it would make as much sense to deprecate that one by itself.

ThreadedChildWatcher starts a thread per process but has O(1) complexity.

MultiLoopChildWatcher doesn't spawn threads, it can be used safely with asyncio loops spawn in multiple threads. The complexity is O(N) plus no other code should contest for SIG_CHLD subscription.

FastChildWatcher has O(1), this is the good news. All others are bad: the watcher conflicts even with blocking subprocess.wait() call, even if the call is performed from another thread.

SafeChildWatcher is safer than FastChildWatcher but working with asyncio subprocess API is still super complicated if asyncio code is running from multiple threads. SafeChildWatcher works well only if asyncio is run from the main thread only. Complexity is O(N).

I think FastChildWatcher and SafeChildWatcher should go, ThreadedChildWatcher should be kept default and MultiLoopChildWatcher is an option where ThreadedChildWatcher is not satisfactory.

MultiLoopChildWatcher problems can and should be fixed; there is nothing bad in the idea but slightly imperfect implementation.

Regarding pidfd and kqueue -- I love to see pull requests with proposals. Now nothing exists.
The pidfd is available starting from the latest released Linux 5.3; we need to wait for a decade before all Linux distros adopt it and we can drop all other implementations.

I think FastChildWatcher and SafeChildWatcher should go, ThreadedChildWatcher should be kept default and MultiLoopChildWatcher is an option where ThreadedChildWatcher is not satisfactory.

Okay, I think I can understand the reasoning here. Do you think that FastChildWatcher and SafeChildWatcher could be deprecated starting in 3.9 and removed in 3.11? If so, I'd be glad to start working on adding the deprecation warnings and the 3.9 Whats New entries.

MultiLoopChildWatcher problems can and should be fixed; there is nothing bad in the idea but slightly imperfect implementation.

Yeah my largest concern was just that the current issues seem especially complex to fix and I interpreted your previous comment as "I'd like to deprecate all the process watchers in the near future". Thus, it seemed to make sense to me that we could start removing them rather than sinking time into fixing something that might be removed soon.

By "slightly imperfect implementation", do you have any ideas for particularly imperfect parts of it that could use improvement?

I feel that I've developed a decent understanding of the implementation for ThreadedChildWatcher, but after looking at the race conditions for MultiLoopChildWatcher in https://bugs.python.org/issue38323, I'll admit that I felt a bit lost for where to find a solution. Primarily because my understanding of the signal module is quite limited in comparison to others; it's not an area that I've strongly focused on.

ThreadedChildWatcher starts a thread per process but has O(1) complexity.

But it spawns a new Python thread per process which can be a blocker issue if a server memory is limited. What if you want to spawn 100 processes? Or 1000 processes? What is the memory usage?

I like FastChildWatcher!

... but working with asyncio subprocess API is still super complicated if asyncio code is running from multiple threads

Well, I like the ability to choose the child watcher implementation depending on my use case. If asyncio is only run from the main thread, FastChildWatcher is safe, fast and has low memory footprint, no?

Regarding pidfd and kqueue -- I love to see pull requests with proposals. Now nothing exists. The pidfd is available starting from the latest released Linux 5.3; we need to wait for a decade before all Linux distros adopt it and we can drop all other implementations.

I hope that it will take less time to expose pidfd_open() in Python! It is *already* available in the Linux kernel 5.3. My laptop is already running Linux 5.3! (Thanks Fedora 30.)

I would prefer to keep an API to choose the child watcher since it seems like even in 2019, there are still new APIs (pidfd) to wait for a process completion. I expect that each implementation will have advantages and drawbacks.

So are we at least in agreement for starting with deprecating FastChildWatcher?

I think so. It will take a long before we remove it though.

I think so. It will take a long before we remove it though.

In that case, it could be a long term deprecation notice, where we start the deprecation process without having a definitive removal version. This will at least encourage users to look towards using the other watchers instead of FastChildWatcher. I can start working on a PR.

This should probably be labeled as 3.12

Duplicate of #94597

This is now tracked in #94597 which contains more recent info.

Navigation Menu

Repository navigation

Deprecate Process Child Watchers #82772

9 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Deprecate Process Child Watchers #82772

Description

Activity

aeros commented on Oct 25, 2019

1st1 commented on Oct 25, 2019

aeros commented on Oct 25, 2019

1st1 commented on Oct 25, 2019

aeros commented on Oct 25, 2019

1st1 commented on Oct 25, 2019

aeros commented on Oct 25, 2019

asvetlov commented on Oct 25, 2019

aeros commented on Oct 25, 2019

vstinner commented on Oct 26, 2019

vstinner commented on Oct 26, 2019

9 remaining items

asvetlov commented on Nov 15, 2019

aeros commented on Nov 15, 2019

graingert commented on Jul 6, 2022

kumaraditya303 commented on Oct 9, 2022

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions