'NoneType' object is not callable in subprocess.py

[ita1024]

BPO	29174
Nosy	@vstinner, @vadmium
Files	test.py: testcase test2.py

^{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 = None
closed_at = <Date 2018-10-28.17:10:06.297>
created_at = <Date 2017-01-06.07:48:58.389>
labels = ['type-bug', 'library']
title = "'NoneType' object is not callable in subprocess.py"
updated_at = <Date 2018-10-28.17:10:06.296>
user = 'https://bugs.python.org/ita1024'

bugs.python.org fields:

activity = <Date 2018-10-28.17:10:06.296>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-28.17:10:06.297>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2017-01-06.07:48:58.389>
creator = 'ita1024'
dependencies = []
files = ['46171', '46172']
hgrepos = []
issue_num = 29174
keywords = []
message_count = 10.0
messages = ['284798', '284804', '284807', '284867', '284881', '284883', '284912', '284934', '284992', '328712']
nosy_count = 4.0
nosy_names = ['vstinner', 'python-dev', 'martin.panter', 'ita1024']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29174'
versions = ['Python 3.6']

[ita1024]

Please try the attached testcase with python3.6 test.py; Python 3.6 displays unnecessary warnings of the following form:

$ ../test.py
Exception ignored in: <bound method Popen.__del__ of <subprocess.Popen object at 0x7fe6744fc5c0>>
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/subprocess.py", line 761, in __del__
TypeError: 'NoneType' object is not callable
Exception ignored in: <bound method Popen.__del__ of <subprocess.Popen object at 0x7fe6744fc550>>
Traceback (most recent call last):
  File "/usr/local/lib/python3.6/subprocess.py", line 761, in __del__

These warnings appear because of the line "warnings.warn" recently added in subprocess.Popen.del:

1. The call to warnings.warn is not usable during interpreter shutdown (and running python -W ignore test.py has no effect)
2. Calling "process.terminate()" or "process.kill()" at in the testcase or in an atexit handler would not get rid of the warning, one must set the return code on the Popen object
3. The warning can show up in existing code that has absolutely no zombie problems.

I suggest to revert the recently added warning from subprocess.py:

"""
@@ -754,11 +995,6 @@
         if not self._child_created:
             # We didn't get to successfully create a child process.
             return
-        if self.returncode is None:
-            # Not reading subprocess exit status creates a zombi process which
-            # is only destroyed at the parent python process exit
-            warnings.warn("subprocess %s is still running" % self.pid,
-                          ResourceWarning, source=self)
         # In case the child hasn't been waited on, check if it's done.
         self._internal_poll(_deadstate=_maxsize)
         if self.returncode is None and _active is not None:
"""

[python-dev]

New changeset b14a1e81c34a by Victor Stinner in branch '3.6':
Fix subprocess.Popen.__del__() fox Python shutdown
https://hg.python.org/cpython/rev/b14a1e81c34a

[vstinner]

1. The call to warnings.warn is not usable during interpreter shutdown (and running python -W ignore test.py has no effect)

Oops right. I just fixed this issue.

2. Calling "process.terminate()" or "process.kill()" at in the testcase or in an atexit handler would not get rid of the warning, one must set the return code on the Popen object

Hum. I should document somehow how to fix such bug: you must read the exit status of the child process, not set manually the returncode attribute. You have to call the wait() method of each process after calling terminate().

3. The warning can show up in existing code that has absolutely no zombie problems.

I modified your example to list zombi processes: try test2.py.

Output:

$ ./python test2.py 
0  1000 25520 25120  20   0 140940 11828 wait   S+   pts/0      0:00 ./python test2.py
0  1000 25521 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25522 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25523 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25524 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25525 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25526 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25527 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25528 25520  20   0      0     0 -      Z+   pts/0      0:00 [python] <defunct>
0  1000 25529 25520  20   0 119032  3008 wait   S+   pts/0      0:00 sh -c ps l|grep 25520
0  1000 25531 25529  20   0 118540   880 -      S+   pts/0      0:00 grep 25520
Lib/subprocess.py:761: ResourceWarning: subprocess 25528 is still running
sys:1: ResourceWarning: unclosed file <_io.FileIO name=18 mode='wb' closefd=True>
sys:1: ResourceWarning: unclosed file <_io.FileIO name=19 mode='rb' closefd=True>
(...)

The long list of <defunct> are the zombi processes: it means that the kernel is unable to remove completely child processes because the parent didn't read the exit status yet. Process identifiers and memory are wasted.

The warning can also help to detect when an application forgot to check the exit status. At least, if you add a call to process.wait(), it becomes explicit that ignoring the exit status is deliberate.

[ita1024]

The point #3 was referring to the new requirement for an atexit handler in order to not only kill the processes but to also wait for them at interpreter shutdown. The sub-processes (and associated resources) in the example are definitely freed as the parent process is terminating.

The recommended handler is not even always desirable (spawning daemon processes, key agents), it increases the code verbosity, impacts performance, and can even cause problems as child processes cannot always be waited on reliably (python 2 but also child -D state and platform-specific restrictions).

I suggest to disable such warnings during interpreter shutdown.

[vadmium]

The ResourceWarning was added by bpo-26741.

I agree that there are legitimate reasons why pre-3.6 code may avoid calling Popen.wait() and equivalent. Victor opened bpo-27068 about adding a Popen.detach() method, which such code could use to opt out of the warning.

I don’t think there should be a special exemption for the warning at shutdown time. I think we should either:

1. Accept that you should never destroy a 3.6 Popen object without first “waiting” on its child (or zombie), or:

2. Revert the warning, and in a future release (e.g. 3.7), add it back along with a way to opt out of the warning.

[vstinner]

Martin Panter:

Victor opened bpo-27068 about adding a Popen.detach() method, which such code could use to opt out of the warning.

I opened the issue because you asked me to open it, but I'm not
convinced yet that the design would work. I don't understand yet who
is responsible of the pipes for example, especially pipes opened by
the Popen object itself (ex: stdout=PIPE), not passed to Popen
constructor. It's not as simple as getting a file descriptor as
file.detach() or socket.detach(), a Popen object is made of multiple
resources (pid and pipes at least).

2. Revert the warning, and in a future release (e.g. 3.7), add it back along with a way to opt out of the warning.

For this specific issue, the ResourceWarning is correct. I don't
understand the use case of explicitly turning this warning off on this
specific example?

If your output is flooded by ResourceWarning warnings, it's easy to
configure Python to ignore them. Example, simplest option: python3
-Wignore script.py. But you are only going to hide a real issue in
your code. ResourceWarning exists to help you to reduce your resource
consumption.

[ita1024]

For this specific issue, the ResourceWarning is correct. I don't
understand the use case of explicitly turning this warning off on this
specific example?

No, on this specific example the child processes do terminate properly as the parent process terminate. There is no extra resource consumption so the warning is incorrect and annoying.

If your output is flooded by ResourceWarning warnings, it's easy to
configure Python to ignore them. Example, simplest option: python3
-Wignore script.py. But you are only going to hide a real issue in
your code. ResourceWarning exists to help you to reduce your resource
consumption.

At the moment the outputs are flooded with "exception ignored" messages that show how well this was thought out. Asking users to add -Wignore options for perfectly legitimate applications also shows how much consideration is being given to user experience, but we should not be surprised because that is how Python3 is being developed?

Please remove this mess until a proper design is made and documented (3.7).

[vadmium]

The code in test.py is not realistic. It spawns children only to terminate them straight away, and you could easily reap each child after calling terminate(). You might have more influence with a realistic use case.

Victor has committed a fix for the “exception ignored” problem, so assuming it works, in the next release of Python it should be gone. You should still see a ResourceWarning if warnings are enabled, but I don’t think -Wignore would be necessary to suppress them; such warnings are disabled by default.

[ita1024]

The code in test.py is not realistic.

My application uses a process pool that gets freed when my application terminates.

You might have more influence with a realistic use case.

I am reporting this because the users of my application are complaining.

Victor has committed a fix for the “exception ignored” problem, so assuming it works

I am looking forward to seeing that. Thanks Victor!

[vstinner]

This bug is now fixed if I understood correctly.