[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Jack O'Connor]

Jack O'Connor  added the comment:

I'm late to the party, but I want to explain what's going on here in case it's 
helpful to folks. The issue you're seeing here has to do with whether a child 
processs has been "reaped". (Windows is different from Unix here, because the 
parent keeps an open handle to the child, so this is mostly a Unix thing.) In 
short, when a child exits, it leaves a "zombie" process whose only job is to 
hold some metadata and keep the child's PID reserved.  When the parent calls 
wait/waitpid/waitid or similar, that zombie process is cleaned up. That means 
that waiting has important correctness properties apart from just blocking the 
parent -- signaling after wait returns is unsafe, and forgetting to wait also 
leaks kernel resources.

Here's a short example demonstrating this:

```
  import signal 

   
  import subprocess 

   
  import time   

   


   
  # Start a child process and sleep a little bit so that we know it's exited.   

   
  child = subprocess.Popen(["true"])

   
  time.sleep(1) 

   


   
  # Signal it. Even though it's definitely exited, this is not an error.

  
  os.kill(child.pid, signal.SIGKILL)

   
  print("signaling before waiting works fine")  

   


   
  # Now wait on it. We could also use os.waitpid or os.waitid here. This reaps  

   
  # the zombie child.   

   
  child.wait()  

   


   
  # Try to signal it again. This raises ProcessLookupError, because the child's 

   
  # PID has been freed. But note that Popen.kill() would be a no-op here,
  # because it knows the child has already been waited on.  

  
  os.kill(child.pid, signal.SIGKILL)

   
```

With that in mind, the original behavior with communicate() that started this 
bug is expected. The docs say that communicate() "waits for process to 
terminate and sets the returncode attribute." That means internally it calls 
waitpid, so your terminate() thread is racing against process exit. Catching 
the exception thrown by terminate() will hide the problem, but the 

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[miss-islington]

miss-islington  added the comment:


New changeset 713b4bbd97392acc492a4fefb2d07ae61fb7b75d by Miss Islington (bot) 
in branch '3.9':
bpo-40550: Fix time-of-check/time-of-action issue in 
subprocess.Popen.send_signal. (GH-20010)
https://github.com/python/cpython/commit/713b4bbd97392acc492a4fefb2d07ae61fb7b75d


--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Thanks for the patch!

PRs are in or on their way in for 3.10 and 3.9.

The 3.8 auto-backport failed, if anyone wants it on a future 3.8.x please 
follow up with a manual cherry pick to make a PR for the 3.8 branch.

--
resolution:  -> fixed
stage: patch review -> commit review
status: open -> closed
versions:  -Python 3.8

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 3.0 -> 4.0
pull_requests: +22331
pull_request: https://github.com/python/cpython/pull/23439

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset 01a202ab6b0ded546e47073db6498262086c52e9 by Filipe Laíns in 
branch 'master':
bpo-40550: Fix time-of-check/time-of-action issue in 
subprocess.Popen.send_signal. (GH-20010)
https://github.com/python/cpython/commit/01a202ab6b0ded546e47073db6498262086c52e9


--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Gregory P. Smith]

Change by Gregory P. Smith :


--
assignee:  -> gregory.p.smith
nosy: +gregory.p.smith
versions: +Python 3.10, Python 3.9

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Alexander Overvoorde]

Alexander Overvoorde  added the comment:

I understand that it's not a perfect solution, but at least it's a little bit 
closer. Thanks for your patch :)

--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Filipe Laíns]

Filipe Laíns  added the comment:

I submitted a patch. As explained above, this only mitigates the 
time-of-check/time-of-action issue in case the process doesn't exist, it *is 
not* a proper fix for the issue. But don't see any way to properly fix it.

--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Filipe Laíns]

Change by Filipe Laíns :


--
keywords: +patch
pull_requests: +19321
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/20010

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Filipe Laíns]

Filipe Laíns  added the comment:

This is a simple time-of-check - time-of-action issue, which is why I suggested 
that it shouldn't be fixed. I was not aware send_signal did have this check, 
which tells us it is supposed to be suported(?). Anyway, to fix it we need to 
catch the exception and check for errno == 3 so that we can ignore it.

Optimally we would want to have an atomic operation here, but no such thing 
exists. There is still the very faint possibility that after your process exits 
a new process will take its id and we kill it instead.

We should keep the returncode check and just ignore the exception when errno == 
3. This is the best option.

--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Alexander Overvoorde]

Alexander Overvoorde  added the comment:

I'm not sure that it is expected since Popen.send_signal does contain the 
following check:

```
def send_signal(self, sig):
"""Send a signal to the process."""
# Skip signalling a process that we know has already died.
if self.returncode is None:
os.kill(self.pid, sig)
```

Additionally, the following program does not raise a ProcessLookupError despite 
the program already having exited:

```
import subprocess
import time

proc = subprocess.Popen(["sh", "-c", "exit 0"])

time.sleep(5)

proc.terminate()
```

--

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Filipe Laíns]

Filipe Laíns  added the comment:

This is the backtrace I get:

Traceback (most recent call last):
  File "/home/anubis/test/multiprocessing-error.py", line 16, in 
proc.terminate()
  File "/home/anubis/git/cpython/Lib/subprocess.py", line 2069, in terminate
self.send_signal(signal.SIGTERM)
  File "/home/anubis/git/cpython/Lib/subprocess.py", line 2064, in send_signal
os.kill(self.pid, sig)
ProcessLookupError: [Errno 3] No such process

Is yours the same? This is expected, the process exited before proc.terminate().

You should wrap proc.terminate() in a try..except block:

try:
proc.terminate()
except ProcessLookupError:
pass

I am not sure we want to suppress this.

--
nosy: +FFY00

___
Python tracker 

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

[issue40550] Popen.terminate fails with ProcessLookupError under certain conditions

[Alexander Overvoorde]

New submission from Alexander Overvoorde :

The following program frequently raises a ProcessLookupError exception when 
calling proc.terminate():

```
import threading
import subprocess
import multiprocessing

proc = subprocess.Popen(["sh", "-c", "exit 0"])

q = multiprocessing.Queue()

def communicate_thread(proc):
proc.communicate()

t = threading.Thread(target=communicate_thread, args=(proc,))
t.start()

proc.terminate()

t.join()
```

I'm reproducing this with Python 3.8.2 on Arch Linux by saving the script and 
rapidly executing it like this:

$ bash -e -c "while true; do python3 test.py; done

The (unused) multiprocessing.Queue seems to play a role here because the 
problem vanishes when removing that one line.

--
components: Library (Lib)
messages: 368370
nosy: Alexander Overvoorde
priority: normal
severity: normal
status: open
title: Popen.terminate fails with ProcessLookupError under certain conditions
type: behavior
versions: Python 3.8

___
Python tracker 

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