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

Swap plugin cache to pickle-able values when done #7640

Merged
merged 9 commits into from Oct 20, 2022
7 changes: 7 additions & 0 deletions 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.
11 changes: 9 additions & 2 deletions pylint/lint/pylinter.py
Expand Up @@ -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
Expand Down Expand Up @@ -402,7 +402,14 @@ 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
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
# 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."""
Expand Down
18 changes: 18 additions & 0 deletions tests/test_check_parallel.py
Expand Up @@ -231,6 +231,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, dill.PickleError):
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
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())
Expand Down