From 51f32bbe93497143b204b95b7ca5513e61754fef Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 28 Oct 2019 09:17:59 -0700 Subject: [PATCH] Only use multiprocessing when the method is fork In python3.8 on macos and in all versions on windows the multiprocessing method is `spawn` which does not preserve class state. --- docs/source/internal/utils.rst | 6 ------ src/flake8/checker.py | 28 ++++++++++++++-------------- src/flake8/utils.py | 24 ------------------------ tests/unit/test_checker_manager.py | 2 +- 4 files changed, 15 insertions(+), 45 deletions(-) diff --git a/docs/source/internal/utils.rst b/docs/source/internal/utils.rst index fed9a47..e104d62 100644 --- a/docs/source/internal/utils.rst +++ b/docs/source/internal/utils.rst @@ -59,12 +59,6 @@ allows plugins to use this to retrieve ``stdin`` if necessary. This provides a convenient and explicitly named function that checks if we are currently running on a Windows (or ``nt``) operating system. -.. autofunction:: flake8.utils.can_run_multiprocessing_on_windows - -This provides a separate and distinct check from -:func:`~flake8.utils.is_windows` that allows us to check if the version of -Python we're using can actually use multiprocessing on Windows. - .. autofunction:: flake8.utils.is_using_stdin Another helpful function that is named only to be explicit given it is a very diff --git a/src/flake8/checker.py b/src/flake8/checker.py index 59064d9..a454f2f 100644 --- a/src/flake8/checker.py +++ b/src/flake8/checker.py @@ -4,6 +4,7 @@ import errno import itertools import logging import signal +import sys import tokenize from typing import Dict, List, Optional, Tuple @@ -35,6 +36,18 @@ SERIAL_RETRY_ERRNOS = { } +def _multiprocessing_is_fork(): # type () -> bool + """Class state is only preserved when using the `fork` strategy.""" + if sys.version_info >= (3, 4): + return ( + multiprocessing + # https://github.com/python/typeshed/pull/3415 + and multiprocessing.get_start_method() == "fork" # type: ignore + ) + else: + return multiprocessing and not utils.is_windows() + + class Manager(object): """Manage the parallelism and checker instances for each plugin and file. @@ -101,26 +114,13 @@ class Manager(object): # - we're processing a diff, which again does not work well with # multiprocessing and which really shouldn't require multiprocessing # - the user provided some awful input - if not multiprocessing: + if not _multiprocessing_is_fork(): LOG.warning( "The multiprocessing module is not available. " "Ignoring --jobs arguments." ) return 0 - if ( - utils.is_windows() - and not utils.can_run_multiprocessing_on_windows() - ): - LOG.warning( - "The --jobs option is not available on Windows due to" - " a bug (https://bugs.python.org/issue27649) in " - "Python 2.7.11+ and 3.3+. We have detected that you " - "are running an unsupported version of Python on " - "Windows. Ignoring --jobs arguments." - ) - return 0 - if utils.is_using_stdin(self.arguments): LOG.warning( "The --jobs option is not compatible with supplying " diff --git a/src/flake8/utils.py b/src/flake8/utils.py index 68eff98..5c7232c 100644 --- a/src/flake8/utils.py +++ b/src/flake8/utils.py @@ -298,30 +298,6 @@ def is_windows(): return os.name == "nt" -# NOTE(sigmavirus24): If and when https://bugs.python.org/issue27649 is fixed, -# re-enable multiprocessing support on Windows. -def can_run_multiprocessing_on_windows(): - # type: () -> bool - """Determine if we can use multiprocessing on Windows. - - This presently will **always** return False due to a `bug`_ in the - :mod:`multiprocessing` module on Windows. Once fixed, we will check - to ensure that the version of Python contains that fix (via version - inspection) and *conditionally* re-enable support on Windows. - - .. _bug: - https://bugs.python.org/issue27649 - - :returns: - True if the version of Python is modern enough, otherwise False - :rtype: - bool - """ - is_new_enough_python27 = (2, 7, 11) <= sys.version_info < (3, 0) - is_new_enough_python3 = sys.version_info > (3, 2) - return False and (is_new_enough_python27 or is_new_enough_python3) - - def is_using_stdin(paths): # type: (List[str]) -> bool """Determine if we're going to read from stdin. diff --git a/tests/unit/test_checker_manager.py b/tests/unit/test_checker_manager.py index 2a6998e..28087fe 100644 --- a/tests/unit/test_checker_manager.py +++ b/tests/unit/test_checker_manager.py @@ -34,7 +34,7 @@ def test_oserrors_cause_serial_fall_back(): assert serial.call_count == 1 -@mock.patch('flake8.utils.is_windows', return_value=False) +@mock.patch('flake8.checker._multiprocessing_is_fork', return_value=True) def test_oserrors_are_reraised(is_windows): """Verify that unexpected OSErrors will cause the Manager to reraise.""" err = OSError(errno.EAGAIN, 'Ominous message')