[issue38692] add a pidfd child process watcher

[Eryk Sun]

Change by Eryk Sun :


--
Removed message: https://bugs.python.org/msg405695

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Eryk Sun]

Change by Eryk Sun :


--
components: +asyncio -Build
nosy: +asvetlov, yselivanov -ahmedsayeed1982
versions: +Python 3.9 -Python 3.11

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Ahmed Sayeed]

Ahmed Sayeed  added the comment:

--8<---
 1 size_t fwrite(const void * __restrict ptr, size_t size, 
http://www-look-4.com/category/travel/
 2   size_t nmemb, register FILE * __restrict stream)
 3 {
 4 size_t retval; https://komiya-dental.com/category/technology/
 5 __STDIO_AUTO_THREADLOCK_VAR;
 6  http://www.iu-bloomington.com/category/technology/
 7 >   __STDIO_AUTO_THREADLOCK(stream);
 8 
 9 retval = fwrite_unlocked(ptr, size, nmemb, stream);
10  https://waytowhatsnext.com/category/technology/
11 __STDIO_AUTO_THREADUNLOCK(stream);
12  http://www.wearelondonmade.com/category/travel/
13 return retval;
14 }
-->8---
 http://www.jopspeech.com/category/travel/
Here, we are at line 7. Using the "next" command leads no where. However,
setting a breakpoint on line 9 and issuing "continue" works.
http://joerg.li/category/travel/
Looking at the assembly instructions reveals that we're dealing with the
critical section entry code http://the-hunters.org/technology/meta-symbol/ [1] 
that should never be interrupted, in this
case by the debugger's implicit breakpoints: http://connstr.net/category/travel/

--8<---
  ... http://embermanchester.uk/category/travel/
1 add_s   r0,r13,0x38
2 mov_s   r3,1
3 llock   r2,[r0]<-.
4 brne.nt r2,0,14 --.  | http://www.slipstone.co.uk/category/travel/
5 scond   r3,[r0]   |  |
6 bne -10 --|--'
7 brne_s  r2,0,84 <-' http://www.logoarts.co.uk/category/travel/
  ...
-->8---
 http://www.acpirateradio.co.uk/category/travel/
Lines 3 until 5 (inclusive) are supposed to be executed atomically. Therefore, 
http://fishingnewsletters.co.uk/crypto/kelleci-bay/
GDB should never (implicitly) insert a breakpoint on lines 4 and 5, else the 
http://www.compilatori.com/category/travel/ 
program will try to acquire the lock again by jumping back to line 3 and
gets stuck in an infinite loop. https://www.webb-dev.co.uk/category/technology/

The solution is to make GDB aware of these patterns so it inserts breakpoints
after the sequence -- line 6 in this example. The solution is to make GDB aware 
of these patterns so it inserts breakpoints 
http://www.go-mk-websites.co.uk/services/kyoto/
after the sequence -- line 6 in this example.
 The solution is to make GDB aware of these patterns so it inserts breakpoints
after the sequence -- line 6 in this example. 
http://www.mconstantine.co.uk/services/new-zealand/
The solution is to make GDB aware of these patterns so it inserts breakpoints
after the sequence -- line 6 in this example

--
components: +Build -asyncio
nosy: +ahmedsayeed1982 -aeros, asvetlov, benjamin.peterson, gousaiyang, 
hroncok, miss-islington, nanjekyejoannah, njs, yselivanov
versions: +Python 3.11 -Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Chih-Hsuan Yen]

Change by Chih-Hsuan Yen :


--
nosy:  -yan12125

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Saiyang Gou]

Change by Saiyang Gou :


--
nosy: +gousaiyang
nosy_count: 10.0 -> 11.0
pull_requests: +24233
pull_request: https://github.com/python/cpython/pull/25515

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

> I can confirm it makes test_posix pass inside a systemd-nspawn container on 
> Arch Linux.

Great!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Chih-Hsuan Yen]

Chih-Hsuan Yen  added the comment:

Thank you very much for the commit "Skip test_posix.test_pidfd_open() on 
EPERM"! I can confirm it makes test_posix pass inside a systemd-nspawn 
container on Arch Linux. On the other hand, I found that tests were broken as 
both systemd libseccomp on Arch Linux were out-dated. With recently-pushed 
systemd 243.162-2 and locally-updated libseccomp 2.4.2, os.pidfd_open() works 
fine. Apparently [1] and [2] are relevant fixes.

By the way, upgrading systemd and libseccomp also fixes 
test_signal.PidfdSignalTest [3], which is added two days ago [4].

[1] 
https://github.com/systemd/systemd-stable/commit/51ea58a04b1851b31a14dfa7eec76254f5ddef16
[2] 
https://github.com/seccomp/libseccomp/commit/be65b26b67099be2b2b4890d736dbd1ad15adf36
[3] https://github.com/python/cpython/pull/17290#issuecomment-556869679
[4] https://bugs.python.org/issue38712

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

I pushed my "Skip test_posix.test_pidfd_open() on EPERM" change just for 
practical reasons. We can hope that Linux sandboxes will shortly be updated to 
allow pidfd_open() syscall. It should be safe for most use cases ;-) I close 
again the issue.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 3ab479a2d1959923c9ab80c227dd1f39720b4e2d by Victor Stinner in 
branch 'master':
bpo-38692: Skip test_posix.test_pidfd_open() on EPERM (GH-17290)
https://github.com/python/cpython/commit/3ab479a2d1959923c9ab80c227dd1f39720b4e2d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

As I already proposed previously, I proposed PR 17290 to simply skip 
test_pidfd_open() if os.pidfd_open() fails with a PermissionError.

I don't propose to change os.pidfd_open(), only the test unit for practical 
reasons: not bother users who have to use a strict Linux sandbox using a 
syscalls whitelist.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +16783
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/17290

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

I reopen the issue since the discussion restarted.

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

> BTW my kernel is: 5.3.7-301.fc31.x86_64

I get a different error on Fedora 31 with Linux kernel 5.3.9-300.fc31.x86_64:

$ ./python
Python 3.9.0a1+ (heads/method_freelist:e34fa9b8d7, Nov 20 2019, 12:09:54) 
[GCC 9.2.1 20190827 (Red Hat 9.2.1-1)] on linux
>>> import os
>>> os.pidfd_open(-1)
Traceback (most recent call last):
  File "", line 1, in 
OSError: [Errno 22] Invalid argument

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Nathaniel Smith]

Nathaniel Smith  added the comment:

I don't know about podman, but it sounds like mock and docker both use buggy 
sandboxing: https://bugzilla.redhat.com/show_bug.cgi?id=1770154

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Miro Hrončok]

Miro Hrončok  added the comment:

BTW my kernel is: 5.3.7-301.fc31.x86_64

Sorry for commenting twice, I forgot to mention that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Miro Hrončok]

Miro Hrončok  added the comment:

I have consistent behavior on Fedora 32 in mock [0] and podman [1]. Wanted to 
test docker as well, but my docker setup is currently broken.

# python3.9
Python 3.9.0a1 (default, Nov 20 2019, 00:00:00) 
[GCC 9.2.1 20190827 (Red Hat 9.2.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.pidfd_open(-1)
Traceback (most recent call last):
  File "", line 1, in 
PermissionError: [Errno 1] Operation not permitted


=> the test fails.


[0] https://github.com/rpm-software-management/rpm
[1] https://podman.io/

--
nosy: +hroncok

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Change by Kyle Stanley :


--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[miss-islington]

miss-islington  added the comment:


New changeset 3f8cebd32c1e6f20a1a1440e51c308a5f70ca959 by Miss Islington (bot) 
(Kyle Stanley) in branch 'master':
bpo-38692: Add asyncio.PidfdChildWatcher to __all__ (GH-17161)
https://github.com/python/cpython/commit/3f8cebd32c1e6f20a1a1440e51c308a5f70ca959


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Change by Kyle Stanley :


--
pull_requests: +16671
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/17161

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> PidfdChildWatcher should be enumerated in unix_events.py:__all__ to make the 
> class visible by asyncio import rules.

> Kyle, would you make a post-fix PR?

I actually just noticed that myself and was coming back to the bpo issue to 
mention that it was missing from __all__. I'll take of that, no problem.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Sorry for the late review.

PidfdChildWatcher should be enumerated in unix_events.py:__all__ to make the 
class visible by asyncio import rules.

Kyle, would you make a post-fix PR?

--
resolution: fixed -> 
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset 3ccdd9b180f9a3f29c8ddc8ad1b331fe8df26519 by Benjamin Peterson in 
branch 'master':
closes bpo-38692: Add a pidfd child process watcher to asyncio. (GH-17069)
https://github.com/python/cpython/commit/3ccdd9b180f9a3f29c8ddc8ad1b331fe8df26519


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Thanks everyone for the reviews. I will go ahead and merge the PR.

Shall we return to #38591 for planning the future of child watching?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> > The child watchers API has to go. It's confusing, painful to use, it's not 
> > compatible with third-party event loops. It increases the API surface 
> > without providing us with enough benefits.

> +1

Also, adding to this, the child watchers are one of the least used components 
of asyncio's public API. So, I think the deprecation and removal cost will be 
fairly minimal. See the GitHub code usage (includes exact copies of 
Lib/asyncio/unix_events.py, so there's some redundancy):

MultiLoopChildWatcher: 
https://github.com/search?l=Python&q=MultiLoopChildWatcher&type=Code (20 
results, just added in 3.8)
ThreadedChildWatcher: 
https://github.com/search?l=Python&q=ThreadedChildWatcher&type=Code (77 
results, default unix child watcher, rarely used explicitly)
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)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> We can merge this PR as is (Benjamin, thanks for working on this!), but I 
> think that as soon as we merge it we should do some refactoring and 
> deprecations.

> The child watchers API has to go. It's confusing, painful to use, it's not 
> compatible with third-party event loops. It increases the API surface without 
> providing us with enough benefits.

+1

> In 3.9 we will use kqueue / pidfd / threads / winapis -- whatever is 
> available, but we never use SIGCHLD by default.

IIRC, we don't use SIGCHLD by default at the moment, since ThreadedChildWatcher 
is the default child watcher. Should we change the default to be the new 
PidfdChildWatcher for Linux kernels 5.3+ in Python 3.9, and fallback to 
ThreadedChildWatcher for systems without pidfd_open()?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Yury Selivanov]

Yury Selivanov  added the comment:

We can merge this PR as is (Benjamin, thanks for working on this!), but I think 
that as soon as we merge it we should do some refactoring and deprecations.

The child watchers API has to go. It's confusing, painful to use, it's not 
compatible with third-party event loops. It increases the API surface without 
providing us with enough benefits.

What I propose:

* merge this watcher and try to use it as the default on modern Linuxes.  We 
don't document it.

* deprecate add_child_watcher and all child watcher classes in 3.9. Aim for 
removal in 3.11.  

In 3.9 we will use kqueue / pidfd / threads / winapis -- whatever is available, 
but we never use SIGCHLD by default.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

It will be fixed, though, as soon as the user upgrades systemd.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

> Why is that sandbox configuration important enough to handle? It won't be 
> tested by our CI and no one will know whether they can ever remove the EPERM 
> skipping case.

It's just convenient for users who use/test Python in a Linux sandbox.

The fact is that 2 days after you merged the new test, a first user reported a 
failure. I expect more issues if we don't simply fix the test :-) Sandboxes on 
Linux are more and more common.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Sure, that change on it's own looks small and harmless. My point is that it's a 
slippery slope. Why is that sandbox configuration important enough to handle? 
It won't be tested by our CI and no one will know whether they can ever remove 
the EPERM skipping case.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

> We should not claim to support running our tests in weird syscall sandboxes. 
> There's an infinite number of possible sandboxing configurations, and we 
> can't fix them all.

There is no request to support an "an infinite number of possible sandboxing 
configurations". Only to skip the test if the syscall fails with EPERM. That 
sounds reasonable to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

We should not claim to support running our tests in weird syscall sandboxes. 
There's an infinite number of possible sandboxing configurations, and we can't 
fix them all.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

> It seems like systemd-nspawn is just breaking everything: 
> https://sourceware.org/ml/libc-alpha/2019-11/msg00277.html

Well, we can try to argue to not block syscalls, but I'm not sure that we can 
win such battle :-) For os.urandom(), I chose to handle EPERM as ENOSYS in 
bpo-27955. Extract of Python/bootstrap_hash.c:

/* ENOSYS: the syscall is not supported by the kernel.
   EPERM: the syscall is blocked by a security policy (ex: SECCOMP)
   or something else. */
if (errno == ENOSYS || errno == EPERM) {
getrandom_works = 0;
return 0;
}

We can just skip the test if the syscall fails with EPERM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Chih-Hsuan Yen]

Chih-Hsuan Yen  added the comment:

Benjamin Peterson, Kyle Stanley: Thank you both very much for intensive testing 
and helpful information! Yes on my Arch Linux box tests are also fine without 
systemd-nspawn. However, as I have to use systemd-nspawn for some reasons, and 
investigating how it blocks system calls is beyond my ability, I just add a 
workaround for now: 
https://aur.archlinux.org/cgit/aur.git/tree/python-git-issue38692.diff?h=python-git.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

It seems like systemd-nspawn is just breaking everything: 
https://sourceware.org/ml/libc-alpha/2019-11/msg00277.html

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

[aeros:~/repos/benjaminp-cpython]$ ./python -m test test_posix -F  
(asyncio-pidfd) 
...
0:08:52 load avg: 1.89 [1008] test_posix
0:08:52 load avg: 2.22 [1009] test_posix
...

1008 tests OK.

Total duration: 8 min 52 sec
Tests result: INTERRUPTED

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> [aeros:~/repos/benjaminp-cpython]$ ./python -m test test_pty -F
> (asyncio-pidfd) 
...
0:01:31 load avg: 1.57 [2506] test_pty
0:01:31 load avg: 1.57 [2507] test_pty

Oops, looks like I copied the wrong results of a separate test I was running 
earlier. I'll post the results of ~1000 iterations of test_posix below, once it 
is completed again.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> I got a failure in newly added test_pidfd_open:

> I'm running kernel 5.3.7-x86_64-linode130 with Arch Linux.

> I think you must still be experiencing some sort of sandboxing. I don't know 
> how else you would get an EPERM out of pidfd_open.

I believe Benjamin is correct. On a native install of Arch Linux with kernel 
5.3.7 (using latest updates from the official repos), I received no failures in 
test_posix. 

[aeros:~/repos/benjaminp-cpython]$ ./python -m test test_posix 
(asyncio-pidfd) 
0:00:00 load avg: 1.86 Run tests sequentially
0:00:00 load avg: 1.86 [1/1] test_posix

== Tests result: SUCCESS ==

1 test OK.

Total duration: 544 ms
Tests result: SUCCESS

To confirm there weren't intermittent failures, I also ran test_posix 
indefinitely, sending SIGINT after ~2500 iterations. No failures occurred:

[aeros:~/repos/benjaminp-cpython]$ ./python -m test test_pty -F
(asyncio-pidfd) 
...
0:01:31 load avg: 1.57 [2506] test_pty
0:01:31 load avg: 1.57 [2507] test_pty
^C

== Tests result: INTERRUPTED ==
Test suite interrupted by signal SIGINT.

2506 tests OK.

Total duration: 1 min 31 sec
Tests result: INTERRUPTED

It seems that the issue is likely specific to Chih-Hsuan Yen's environment.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

I think you must still be experiencing some sort of sandboxing. I don't know 
how else you would get an EPERM out of pidfd_open.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Chih-Hsuan Yen]

Chih-Hsuan Yen  added the comment:

I got a failure in newly added test_pidfd_open:

==
FAIL: test_pidfd_open (test.test_posix.PosixTester)
--
Traceback (most recent call last):
  File "/build/python-git/src/cpython/Lib/test/test_posix.py", line 1479, in 
test_pidfd_open
self.assertEqual(cm.exception.errno, errno.EINVAL)
AssertionError: 1 != 22

--

I'm running kernel 5.3.7-x86_64-linode130 with Arch Linux. At first I suspect 
that it's related to system call filters from systemd as tests are run in a 
systemd-nspawn container. However, there are still failures after patching 
systemd with 
https://github.com/systemd/systemd/commit/9e486265716963439fb0fd7f2a97abf109f24f75.

How about also skipping the test if pidfd_open returns EPERM?

--
nosy: +yan12125

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

On Wed, Nov 6, 2019, at 09:57, STINNER Victor wrote:
> 
> STINNER Victor  added the comment:
> 
> Would it be useful to use a pidfd in subprocess.Popen to fix bpo-38630 
> root issue, when pidfd is available?

Probably, but as noted above, we need to figure out the best way to integrate 
pidfds into subprocess. (Probably not in this issue.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

Would it be useful to use a pidfd in subprocess.Popen to fix bpo-38630 root 
issue, when pidfd is available?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Change by Benjamin Peterson :


--
pull_requests: +16578
pull_request: https://github.com/python/cpython/pull/17069

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset 6c4c45efaeb40f4f837570f57d90a0b3429c6ae9 by Benjamin Peterson in 
branch 'master':
bpo-38692: Add os.pidfd_open. (GH-17063)
https://github.com/python/cpython/commit/6c4c45efaeb40f4f837570f57d90a0b3429c6ae9


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

pidfd_open was added after pidfd pollling, so it should suffice to make sure 
pidfd_open doesn't ENOSYS.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Nathaniel Smith]

Nathaniel Smith  added the comment:

hasattr is useful for supporting old versions of the os module, but asyncio 
doesn't have to care about that. You should probably try calling pidfd_open and 
see whether you get -ESYSCALL, and also maybe try passing it to poll(), since I 
think there might have been a release where the syscall existed but it wasn't 
pollable. Not sure what the error for that looks like.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Kyle Stanley]

Kyle Stanley  added the comment:

> My the main question is: how to detect if the new watcher can be used or 
> asyncio should fallback to threaded based solution?

Perhaps in the __init__ we could do something like this:

class PidfdChildWatcher(AbstractChildWatcher):

def __init__(self):
if not hasattr(os, "pidfd_open"):
 raise RuntimeError("os.pidfd_open() is unavailable.")
...

Thoughts?

> My WIP progress patch is attached. It passes all asyncio tests.

I think we could also specifically look for race conditions with `./python -m 
test test_asyncio.test_subprocess -F -j4`, rather than relying on the tests 
passing with a single job. The other child watchers have been particularly 
infamous when it comes race conditions and resource leaks. 

I'd be glad to work on testing and opening a PR to asyncio if needed (while 
giving Benjamin credit for the patch of course).

--
nosy: +aeros

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-38630 "subprocess.Popen.send_signal() should poll the process".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Yes, I will be submitting followup changes for pidfd_send_signal and the other 
goodies.

I would like to use pidfds in subprocess, but as you as you say, that's another 
kettle of fish.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Nathaniel Smith]

Nathaniel Smith  added the comment:

Thanks for the CC.

It would be nice to get `pidfd_send_signal` as well, while we're at it. But 
that could be a separate PR.

AFAICT the other bits of the pidfd API right now are some extra flags to clone, 
and an extra flag to waitid. The waitid flag is nice-to-have but not really 
urgent, since it's easy enough to use a flag even if the stdlib doesn't 
explicitly expose it. The clone flags are super interesting, but before we can 
use them, we need to have some way to access clone itself, and that's a big 
complicated project, so we definitely shouldn't worry about it here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

Change by Benjamin Peterson :


--
pull_requests: +16571
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/17063

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

+self._loop._remove_reader(pidfd)
+os.close(pidfd)

Note: Maybe do these operations in the reverse order. I expect a higher risk of 
exception when calling Python self._loop._remove_reader(), than on closing a FD 
that we opened.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +nanjekyejoannah

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[STINNER Victor]

STINNER Victor  added the comment:

I like the idea of exposing pidfd_open() syscall as os.pidfd_open().

Benjamin: Would you mind to create a PR based on your patch, but without your 
asyncio change?

> +#ifndef __NR_pidfd_open

I would prefer to avoid this part of the patch.

You should accept an optional flags parameter, no?

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Nathaniel, you may be interested in the pidfd_open(), at least in adding the 
function to os module.

--
nosy: +njs

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Awesome!

I think the patch can be splitted in os.pidfd_open() and asyncio part itself.

os modification is clear.

asyncio part looks correct after the brief view. 
My the main question is: how to detect if the new watcher can be used or 
asyncio should fallback to threaded based solution?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38692] add a pidfd child process watcher

[Benjamin Peterson]

New submission from Benjamin Peterson :

Recent versions of Linux has built out support for pidfd, a way to do process 
management with file descriptors. I wanted to try it out, so implemented a 
pidfd-based child watcher for asyncio.

My WIP progress patch is attached. It passes all asyncio tests.

--
components: asyncio
files: 0001-asyncio-Add-a-pidfd-child-process-watcher.patch
keywords: patch
messages: 356001
nosy: asvetlov, benjamin.peterson, yselivanov
priority: normal
severity: normal
status: open
title: add a pidfd child process watcher
type: enhancement
versions: Python 3.9
Added file: 
https://bugs.python.org/file48695/0001-asyncio-Add-a-pidfd-child-process-watcher.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com