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.
14 changes: 12 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,17 @@ 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)
# Mypy is not smart enough to realize that we only call
# this line if the object has the attr we are looking at.
# Hence the one-line type: ignore[union-attr] here.
daogilvie marked this conversation as resolved.
Show resolved Hide resolved
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -11,6 +11,8 @@
import argparse
import multiprocessing
import os
import sys
from pathlib import Path

import dill
import pytest
Expand Down Expand Up @@ -231,6 +233,22 @@ def test_worker_check_single_file_no_checkers(self) -> None:
assert stats.statement == 18
assert stats.warning == 0

def test_linter_with_plugins_is_pickleable(self) -> None:
"""
The linter needs to be pickle-able in order to be passed between workers
"""
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
dummy_plugin_path = Path(__file__).parent / "regrtest_data" / "dummy_plugin"
dummy_plugin_path_str = str(dummy_plugin_path.absolute())
sys.path.append(dummy_plugin_path_str)
linter = PyLinter(reporter=Reporter())
print(linter._dynamic_plugins)
daogilvie marked this conversation as resolved.
Show resolved Hide resolved
try:
dill.dumps(linter)
except dill.PicklingError:
assert False, "Loading plugins caused an un-pickle-able linter"
finally:
sys.path.remove(dummy_plugin_path_str)

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