Skip to content

Commit e8dc9b6

Browse files
Drummond OgilvieDanielNoordcdce8p
committed
Swap plugin cache to pickle-able values when done (#7640)
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 <[email protected]> Co-authored-by: Marc Mueller <[email protected]>
1 parent b051fab commit e8dc9b6

File tree

4 files changed

+37
-3
lines changed

4 files changed

+37
-3
lines changed

doc/whatsnew/fragments/7635.bugfix

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Fixed a multi-processing crash that prevents using any more than 1 thread on MacOS.
2+
3+
The returned module objects and errors that were cached by the linter plugin loader
4+
cannot be reliably pickled. This means that ``dill`` would throw an error when
5+
attempting to serialise the linter object for multi-processing use.
6+
7+
Closes #7635.

pylint/lint/pylinter.py

+10-2
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ def __init__(
296296
str, list[checkers.BaseChecker]
297297
] = collections.defaultdict(list)
298298
"""Dictionary of registered and initialized checkers."""
299-
self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError] = {}
299+
self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError | bool] = {}
300300
"""Set of loaded plugin names."""
301301

302302
# Attributes related to registering messages and their handling
@@ -400,7 +400,15 @@ def load_plugin_configuration(self) -> None:
400400
"bad-plugin-value", args=(modname, module_or_error), line=0
401401
)
402402
elif hasattr(module_or_error, "load_configuration"):
403-
module_or_error.load_configuration(self)
403+
module_or_error.load_configuration(self) # type: ignore[union-attr]
404+
405+
# We re-set all the dictionary values to True here to make sure the dict
406+
# is pickle-able. This is only a problem in multiprocessing/parallel mode.
407+
# (e.g. invoking pylint -j 2)
408+
self._dynamic_plugins = {
409+
modname: not isinstance(val, ModuleNotFoundError)
410+
for modname, val in self._dynamic_plugins.items()
411+
}
404412

405413
def _load_reporters(self, reporter_names: str) -> None:
406414
"""Load the reporters if they are available on _reporters."""

script/.contributors_aliases.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@
438438
},
439439
440440
441-
"name": "Drummond Ogilvie"
441+
"name": "Drum Ogilvie"
442442
},
443443
444444
"mails": [

tests/test_check_parallel.py

+19
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import argparse
1212
import multiprocessing
1313
import os
14+
from pickle import PickleError
1415

1516
import dill
1617
import pytest
@@ -231,6 +232,24 @@ def test_worker_check_single_file_no_checkers(self) -> None:
231232
assert stats.statement == 18
232233
assert stats.warning == 0
233234

235+
def test_linter_with_unpickleable_plugins_is_pickleable(self) -> None:
236+
"""The linter needs to be pickle-able in order to be passed between workers"""
237+
linter = PyLinter(reporter=Reporter())
238+
# We load an extension that we know is not pickle-safe
239+
linter.load_plugin_modules(["pylint.extensions.overlapping_exceptions"])
240+
try:
241+
dill.dumps(linter)
242+
assert False, "Plugins loaded were pickle-safe! This test needs altering"
243+
except (KeyError, TypeError, PickleError, NotImplementedError):
244+
pass
245+
246+
# And expect this call to make it pickle-able
247+
linter.load_plugin_configuration()
248+
try:
249+
dill.dumps(linter)
250+
except KeyError:
251+
assert False, "Cannot pickle linter when using non-pickleable plugin"
252+
234253
def test_worker_check_sequential_checker(self) -> None:
235254
"""Same as test_worker_check_single_file_no_checkers with SequentialTestChecker."""
236255
linter = PyLinter(reporter=Reporter())

0 commit comments

Comments
 (0)