Possible RecursionError during signal cleanup

[encukou]

I've tried running the test suite with Python 3.8.0a3 (on Fedora 30, pytest-cov master, pytest==4.5.0, coverage==4.5.3). While digging into failures I noticed the following race condition.
Solving it doesn't quite fix the tests for me, but I believe it's valid.

cleanup() and the signal handler _signal_cleanup_handler() can enter an infinite recursion if a signal is delivered at the wrong time.

Trace when cleanup is called (with _cleanup_in_progress False and _pending_signal None):

• _cleanup_in_progress = True
• a signal is delivered now
  • _signal_cleanup_handler runs
  • _cleanup_in_progress is True, so the handler only sets _pending_signal
• _cleanup(_active_cov) runs
• _active_cov = None
• _cleanup_in_progress = False
• _pending_signal is set, so we call _signal_cleanup_handler(), with _cleanup_in_progress False and _pending_signal set:
  • _cleanup_in_progress is False, so we call cleanup():
    • _cleanup_in_progress = True
    • _cleanup(_active_cov)
    • _active_cov = None
    • _cleanup_in_progress = False
    • _pending_signal is still set, so call _signal_cleanup_handler(), again with _cleanup_in_progress False and _pending_signal set. This leads to infinite recursion.

The solution could be setting _cleanup_in_progress = False after the if _pending_signal block, or setting _pending_signal = None before calling _signal_cleanup_handler.
I'm not very familiar with the code and what it's trying to do, so I don't know what a good fix would be (if any).

[ionelmc]

Hmmm indeed. This should fix it:

--- a/src/pytest_cov/embed.py
+++ b/src/pytest_cov/embed.py
@@ -97,8 +97,8 @@ def cleanup():
     _active_cov = None
     _cleanup_in_progress = False
     if _pending_signal:
-        _signal_cleanup_handler(*_pending_signal)
         _pending_signal = None
+        _signal_cleanup_handler(*_pending_signal)

The goal of that contrived code was to avoid signal handling to interrupt coverage's shutdown machinery (it's fragile).