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
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 10 additions & 2 deletions pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
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,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
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
2 changes: 1 addition & 1 deletion script/.contributors_aliases.json
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@
},
"[email protected]": {
"mails": ["[email protected]", "[email protected]"],
"name": "Drummond Ogilvie"
"name": "Drum Ogilvie"
},
"[email protected]": {
"mails": [
Expand Down
19 changes: 19 additions & 0 deletions tests/test_check_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import argparse
import multiprocessing
import os
from pickle import PickleError

import dill
import pytest
Expand Down Expand Up @@ -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())
Expand Down