Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove multiprocess finalizer and improve subprocess docs #265

Merged
merged 34 commits into from
Mar 9, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
3160976
Stop using multiprocess finalizers. Better document workarounds for u…
ionelmc Feb 16, 2019
1210cdc
Rename file.
ionelmc Feb 16, 2019
0533855
Update docs/subprocess-support.rst
blueyed Feb 16, 2019
892f4e1
Update docs/subprocess-support.rst
blueyed Feb 16, 2019
b494c37
Update docs/subprocess-support.rst
blueyed Feb 16, 2019
5d49467
Remove the note about windows completely.
ionelmc Feb 16, 2019
1cb8ce6
Well ... bring back the finalizer, but make the cleanup reentrant.
ionelmc Feb 17, 2019
8014767
Remove this attribute and rely on pytest_cov.embed's internal storage…
ionelmc Feb 17, 2019
38c89a8
Ignore SIG_DFL (lil regression).
ionelmc Feb 17, 2019
5374581
Avoid doubly registering the signal handler (so the previous handler …
ionelmc Feb 17, 2019
710e4cc
Add missing global.
ionelmc Feb 17, 2019
8a2bfa8
Add a test for #250.
ionelmc Feb 17, 2019
4046fc2
Add a windows specific test.
ionelmc Feb 17, 2019
65d50cf
Some renaming to better reflect what is actually tested.
ionelmc Feb 17, 2019
49f55d6
Add few more multiprocessing tests and change some details for skips.
ionelmc Feb 17, 2019
b449d92
Skip a bunch of stuff on windows+pypy - it's broken, see https://gith…
ionelmc Feb 18, 2019
c32c158
Correct some assertions. Revert bogus change.
ionelmc Feb 18, 2019
ab9d7bc
Run this fewer times. Maybe travis too slow for such heavy test.
ionelmc Feb 18, 2019
cd0c4a4
Rework a bit the mp pool integration tests to generate a line of code…
ionelmc Feb 21, 2019
3d3b488
Some cleanup.
ionelmc Feb 21, 2019
2291d76
Skip this on windows/pypy (xdist broken).
ionelmc Feb 22, 2019
8f1b9e6
Extend assertion a bit.
ionelmc Feb 22, 2019
478152e
Change the docs again to reflect the current implementation.
ionelmc Feb 22, 2019
7aa50d9
Fix escaping.
ionelmc Feb 23, 2019
3709127
Use travis_wait (mainly for pypy which often times out).
ionelmc Feb 23, 2019
35f38f4
Use travispls instead - travis_wait is so broken ...
ionelmc Feb 23, 2019
bcdce59
Don't run pypy3 on Windows.
ionelmc Feb 23, 2019
c11fe04
Skip the terminate tests on PyPy and remove travispls (doesn't work o…
ionelmc Feb 23, 2019
103d1ef
Remove the automatic SIGTERM handler install from the afterfork
ionelmc Feb 25, 2019
66d8ade
Avoid having stray tracers around. This fixes an "AssertionError: Exp…
ionelmc Feb 25, 2019
65959fc
Avoid writing bogus data files from dead coverage tracers.
ionelmc Feb 25, 2019
fdc43ec
Allow COV_CORE_SOURCE to be empty (it'd be converted to None). Also u…
ionelmc Feb 25, 2019
7557f67
Fix cleanup leaving unusable state.
ionelmc Feb 25, 2019
42f0307
Always skip this on PyPy as it sometimes fail with `error: release un…
ionelmc Mar 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Contents:
reporting
debuggers
xdist
mp
subprocess-support
plugins
markers-fixtures
changelog
Expand Down
70 changes: 0 additions & 70 deletions docs/mp.rst

This file was deleted.

118 changes: 118 additions & 0 deletions docs/subprocess-support.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
==================
Subprocess support
==================

Although pytest-cov supports subprocesses and multiprocessing. However, there are few pitfalls that need to be
ionelmc marked this conversation as resolved.
Show resolved Hide resolved
explained.

Normally coverage writes the data via a pretty standard atexit handler. However, if the subprocess doesn't exit on it's
ionelmc marked this conversation as resolved.
Show resolved Hide resolved
own then the atexit handler might not run. Why that happens is best left to the adventurous to discover by waddling
though the python bug tracker.
ionelmc marked this conversation as resolved.
Show resolved Hide resolved

For now pytest-cov provides opt-in workarounds for these problems.

If you use ``multiprocessing.Pool``
===================================

You need to make sure your ``multiprocessing.Pool`` gets a nice and clean exit:

.. code-block:: python

from multiprocessing import Pool

def f(x):
return x*x

if __name__ == '__main__':
p = Pool(5)
try:
print(p.map(f, [1, 2, 3]))
finally: # <= THIS IS ESSENTIAL
p.close() # <= THIS IS ESSENTIAL
p.join() # <= THIS IS ESSENTIAL

Previously this guide recommended using ``multiprocessing.Pool``'s context manager API, however, that was wrong as
``multiprocessing.Pool.__exit__`` is an alias to ``multiprocessing.Pool.terminate``, and that doesn't always run the
finalizers (sometimes the problem in `cleanup_on_sigterm`_ will appear).

If you use ``multiprocessing.Process``
======================================

There's an identical issue when using the ``Process`` objects. Don't forget to use ``.join()``:

.. code-block:: python

from multiprocessing import Process

def f(name):
print('hello', name)

if __name__ == '__main__':
p = Process(target=f, args=('bob',))
try:
p.start()
finally: # <= THIS IS ESSENTIAL
p.join() # <= THIS IS ESSENTIAL

.. _cleanup_on_sigterm:

If you abuse ``multiprocessing.Process.terminate``
==================================================

It appears that many people are using the ``terminate`` method and then get unreliable coverage results.

On Linux usually that means a SIGTERM gets sent to the process. Unfortunately Python don't have a default handler for
ionelmc marked this conversation as resolved.
Show resolved Hide resolved
SIGTERM so you need to install your own. Because ``pytest-cov`` doesn't want to second-guess (not yet, add your thoughts
on the issue tracker if you disagree) it doesn't install a handler by default, but you can activate it by doing this:

.. code-block:: python

try:
from pytest_cov.embed import cleanup_on_sigterm
except ImportError:
pass
else:
cleanup_on_sigterm()


On Windows there's no nice way to do cleanup (no signal handlers) so you're left to your own devices.
ionelmc marked this conversation as resolved.
Show resolved Hide resolved

If anything else
================

If you have custom signal handling, eg: you do reload on SIGHUP you should have something like this:
ionelmc marked this conversation as resolved.
Show resolved Hide resolved

.. code-block:: python

import os
import signal

def restart_service(frame, signum):
os.exec( ... ) # or whatever your custom signal would do
signal.signal(signal.SIGHUP, restart_service)

try:
from pytest_cov.embed import cleanup_on_signal
except ImportError:
pass
else:
cleanup_on_signal(signal.SIGHUP)

Note that both ``cleanup_on_signal`` and ``cleanup_on_sigterm`` will run the previous signal handler.

Alternatively you can do this:

import os
import signal

try:
from pytest_cov.embed import cleanup
except ImportError:
cleanup = None

def restart_service(frame, signum):
if cleanup is not None:
cleanup()

os.exec( ... ) # or whatever your custom signal would do
signal.signal(signal.SIGHUP, restart_service)
13 changes: 6 additions & 7 deletions src/pytest_cov/embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@


def multiprocessing_start(_):
global active_cov
cov = init()
if cov:
multiprocessing.util.Finalize(None, cleanup, args=(cov,), exitpriority=1000)
active_cov = cov
cleanup_on_sigterm()


try:
Expand Down Expand Up @@ -77,12 +79,9 @@ def _cleanup(cov):
cov.save()


def cleanup(cov=None):
def cleanup():
global active_cov

_cleanup(cov)
if active_cov is not cov:
_cleanup(active_cov)
_cleanup(active_cov)
active_cov = None


Expand All @@ -96,7 +95,7 @@ def _signal_cleanup_handler(signum, frame):
_previous_handler = _previous_handlers.get(signum)
if _previous_handler == signal.SIG_IGN:
return
elif _previous_handler:
elif _previous_handler is not _signal_cleanup_handler:
_previous_handler(signum, frame)
elif signum == signal.SIGTERM:
os._exit(128 + signum)
Expand Down