python / cpython

The Python programming language
https://www.python.org
Other
62.7k stars 30.06k forks source link

subprocess.Popen should not emit a ResourceWarning for a detached process #83071

Open 089d1745-ea22-4f29-b30c-4e658c1dc24d opened 4 years ago

089d1745-ea22-4f29-b30c-4e658c1dc24d commented 4 years ago
BPO 38890
Nosy @gpshead, @pfmoore, @tjguk, @zware, @eryksun, @zooba, @ShaneHarvey
Files
  • test_daemon_win.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: ```python assignee = None closed_at = None created_at = labels = ['type-bug', '3.8', '3.9', '3.10', 'library', 'OS-windows'] title = 'subprocess.Popen should not emit a ResourceWarning for a detached process' updated_at = user = 'https://github.com/ShaneHarvey' ``` bugs.python.org fields: ```python activity = actor = 'gregory.p.smith' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)', 'Windows'] creation = creator = 'ShaneHarvey' dependencies = [] files = ['48738'] hgrepos = [] issue_num = 38890 keywords = [] message_count = 6.0 messages = ['357237', '357451', '357455', '357458', '388906', '389196'] nosy_count = 8.0 nosy_names = ['gregory.p.smith', 'paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'behackett', 'ShaneHarvey'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue38890' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

    089d1745-ea22-4f29-b30c-4e658c1dc24d commented 4 years ago

    In https://bugs.python.org/issue26741 Popen was changed to emit a ResourceWarning in __del__ if the process is still running. However, when running a daemon/detached process it is completely valid to delete a Popen object before the process is complete.

    On Windows, a daemon process can be created by using the DETACHED_PROCESS option, eg: import subprocess

    DETACHED_PROCESS = getattr(subprocess, 'DETACHED_PROCESS', 0x00000008)
    popen = subprocess.Popen(args, creationflags=DETACHED_PROCESS)
    print('Running daemon process: ', popen.pid)
    del popen

    Unfortunately, the above will emit the warning: C:\python\Python37\lib\subprocess.py:839: ResourceWarning: subprocess 840 is still running ResourceWarning, source=self) Running daemon process: 3212

    This makes it complicated to run a daemon process without emitting the resource warning. The best solution I've got is to manually set the Popen.returncode to circumvent the warning, ie:

    popen = subprocess.Popen(args, creationflags=DETACHED_PROCESS)
    print('Running daemon process: ', popen.pid)
    # Set the returncode to avoid erroneous warning:
    # "ResourceWarning: subprocess XXX is still running".
    popen.returncode = 0
    del popen

    Running the attached script on Windows only one warning is emitted: $ python.exe -Wall test_daemon_win.py C:\python\Python37\lib\subprocess.py:839: ResourceWarning: subprocess 3584 is still running ResourceWarning, source=self) Running daemon process: 3584 Running daemon process: 1012

    I propose that Popen should not raise the resource warning when the creationFlags includes DETACHED_PROCESS.

    eryksun commented 4 years ago

    For a console application, normally the system connects to either the console that's inherited from the parent or, if no console is inherited, a newly allocated console. The creation flag DETACHED_PROCESS sets the initial console handle in the child to a special value that tells the system to skip connecting to a console. If the child process doesn't subsequently call AllocConsole, AttachConsole, or create a visible top-level window, then it will considered to be background process.

    Note that 'detached' in this case doesn't mean detached from the parent process. There's no such attachment to begin with because Windows does not maintain a process tree. It uses a nested job-object hierarchy as the equivalent of a Unix process tree. The creation flag CREATE_BREAKAWAY_FROM_JOB breaks away from jobs that allow it.

    The Popen warning is a tool to help programmers become aware of leaked child processes. I do think there should be a documented way to disable this warning for a child process. However, this is unrelated to the DETACHED_PROCESS flag.

    zooba commented 4 years ago

    Didn't we clean up this warning completely on Windows recently?

    It apparently matters on POSIX to join() the child process, but on Windows you aren't going to leak any resources by just forgetting about it, so I thought we stopped tracking them entirely.

    eryksun commented 4 years ago

    Didn't we clean up this warning completely on Windows recently? It apparently matters on POSIX to join() the child process, but on Windows you aren't going to leak any resources by just forgetting about it, so I thought we stopped tracking them entirely.

    There's only a PID in POSIX, so the OS keeps a zombie process until the parent waits on it. Windows uses a handle for the process, which gets automatically closed when the Popen instance is finalized, so we don't have to explicitly wait on the process, and doing so has no bearing on the lifetime of the kernel process object.

    Recently, a change was made to set _active to None in Windows, so _cleanup is no longer called unnecessarily. But Victor wanted to keep the resource warning for its educational value, given some programmers might not be aware when they've left a background process running. So Popen.__del__ still issues a resource warning if returncode is None. The OP's workaround to manually set returncode works, but it's undocumented and feels like a kludge to me.

    eryksun commented 3 years ago

    I wonder if this should be handled more productively by supporting a daemon parameter. In POSIX, use the double fork technique for creating a daemon process. In Windows, use DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP | CREATE_BREAKAWAY_FROM_JOB. For a daemon process, the finalizer would not emit a resource warning or add it to the _active list.

    gpshead commented 3 years ago

    On POSIX the norm for anything daemonizing itself is to fork() and let the parent die so that its original process with the child pid has ended. But I'm used to this being the responsibility of the daemon process. Not the code launching the daemon.

    Related, a deferred PEP on supporting any form of daemon stuff in the stdlib: https://www.python.org/dev/peps/pep-3143/ along with https://pypi.org/project/python-daemon/ and a plethora of other daemon/daemonize related PyPI libraries.

    Those tend to be about implement proper become a daemon behavior in Python processes themselves.

    rubyFeedback commented 2 years ago

    I am running into this issue. I get this warning and I am not sure how to properly clean it so that the warning stops from being issued.

    If possible perhaps python itself could give a recommendation how to deal with it OR the tutorials could explain what to do better. I am kind of looking for a ruby equivalent of system() which I thought I should use "subprocess" in python, but this warning is so distracting ...