From e8dc9b61da9840336277344939141d3a4c4b77df Mon Sep 17 00:00:00 2001 From: Drummond Ogilvie Date: Thu, 20 Oct 2022 10:16:34 +0100 Subject: [PATCH] Swap plugin cache to pickle-able values when done (#7640) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the dictionary has served its purpose (making plugin loading pre-and-post init a consistent behaviour), we swap it for bools indicating whether or not a module was loaded. We don't currently use the bools, but it seemed a sensible choice. The main idea is to make the dictionary fully pickle-able, so that when dill pickles the linter for multiprocessing, it doesn't crash horribly. Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com> Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- doc/whatsnew/fragments/7635.bugfix | 7 +++++++ pylint/lint/pylinter.py | 12 ++++++++++-- script/.contributors_aliases.json | 2 +- tests/test_check_parallel.py | 19 +++++++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 doc/whatsnew/fragments/7635.bugfix diff --git a/doc/whatsnew/fragments/7635.bugfix b/doc/whatsnew/fragments/7635.bugfix new file mode 100644 index 0000000000..72085e0291 --- /dev/null +++ b/doc/whatsnew/fragments/7635.bugfix @@ -0,0 +1,7 @@ +Fixed a multi-processing crash that prevents using any more than 1 thread on MacOS. + +The returned module objects and errors that were cached by the linter plugin loader +cannot be reliably pickled. This means that ``dill`` would throw an error when +attempting to serialise the linter object for multi-processing use. + +Closes #7635. diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index cb528c5df7..47d5baaf52 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -296,7 +296,7 @@ def __init__( str, list[checkers.BaseChecker] ] = collections.defaultdict(list) """Dictionary of registered and initialized checkers.""" - self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError] = {} + self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError | bool] = {} """Set of loaded plugin names.""" # Attributes related to registering messages and their handling @@ -400,7 +400,15 @@ def load_plugin_configuration(self) -> None: "bad-plugin-value", args=(modname, module_or_error), line=0 ) elif hasattr(module_or_error, "load_configuration"): - module_or_error.load_configuration(self) + module_or_error.load_configuration(self) # type: ignore[union-attr] + + # We re-set all the dictionary values to True here to make sure the dict + # is pickle-able. This is only a problem in multiprocessing/parallel mode. + # (e.g. invoking pylint -j 2) + self._dynamic_plugins = { + modname: not isinstance(val, ModuleNotFoundError) + for modname, val in self._dynamic_plugins.items() + } def _load_reporters(self, reporter_names: str) -> None: """Load the reporters if they are available on _reporters.""" diff --git a/script/.contributors_aliases.json b/script/.contributors_aliases.json index 95808f651c..ebd0aa76c3 100644 --- a/script/.contributors_aliases.json +++ b/script/.contributors_aliases.json @@ -438,7 +438,7 @@ }, "me@daogilvie.com": { "mails": ["me@daogilvie.com", "drum.ogilvie@ovo.com"], - "name": "Drummond Ogilvie" + "name": "Drum Ogilvie" }, "me@the-compiler.org": { "mails": [ diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index 3b6d82e04b..a7fb5c158f 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -11,6 +11,7 @@ import argparse import multiprocessing import os +from pickle import PickleError import dill import pytest @@ -231,6 +232,24 @@ def test_worker_check_single_file_no_checkers(self) -> None: assert stats.statement == 18 assert stats.warning == 0 + def test_linter_with_unpickleable_plugins_is_pickleable(self) -> None: + """The linter needs to be pickle-able in order to be passed between workers""" + linter = PyLinter(reporter=Reporter()) + # We load an extension that we know is not pickle-safe + linter.load_plugin_modules(["pylint.extensions.overlapping_exceptions"]) + try: + dill.dumps(linter) + assert False, "Plugins loaded were pickle-safe! This test needs altering" + except (KeyError, TypeError, PickleError, NotImplementedError): + pass + + # And expect this call to make it pickle-able + linter.load_plugin_configuration() + try: + dill.dumps(linter) + except KeyError: + assert False, "Cannot pickle linter when using non-pickleable plugin" + def test_worker_check_sequential_checker(self) -> None: """Same as test_worker_check_single_file_no_checkers with SequentialTestChecker.""" linter = PyLinter(reporter=Reporter())