Fix two multiprocessing issues

[cdce8p]

Description

Fix two multiprocessing issues.

Not possible to enable extension checks with "default_enabled": False

Whenever register_checker is called it overwrites the message filter in case a message is disabled by default. Usually that isn't an issue as the checkers are first registered and the config loaded afterwards.

pylint/pylint/lint/pylinter.py

Lines 495 to 504 in 6ce6032

	def register_checker(self, checker: checkers.BaseChecker) -> None:
	"""This method auto registers the checker."""
	self._checkers[checker.name].append(checker)
	for r_id, r_title, r_cb in checker.reports:
	self.register_report(r_id, r_title, r_cb, checker)
	if hasattr(checker, "msgs"):
	self.msgs_store.register_messages_from_checker(checker)
	for message in checker.messages:
	if not message.default_enabled:
	self.disable(message.msgid)

For multiple jobs the dynamic checkers are re-registered though which overwrites the previously loaded config. To resolve that, it's necessary to restore the message state by caching the list of enabled and disabled messages and reapplying it.

pylint/pylint/lint/parallel.py

Lines 55 to 58 in 6ce6032

	# Re-register dynamic plugins, since the pool does not have access to the
	# astroid module that existed when the linter was pickled.
	_worker_linter.load_plugin_modules(_worker_linter._dynamic_plugins, force=True)
	_worker_linter.load_plugin_configuration()

Closes #10037

Warnings are emitted multiple times

Whenever a checker get's registered, it's just appended to the list of checkers with the corresponding checker name. That's necessary to allow multiple modules to register the same checker name.

pylint/pylint/lint/pylinter.py

Lines 495 to 497 in 6ce6032

	def register_checker(self, checker: checkers.BaseChecker) -> None:
	"""This method auto registers the checker."""
	self._checkers[checker.name].append(checker)

When the checker is now re-registered during the worker init, the "old" one is still active. Thus any message is actually printed twice. To resolve it, deregister the "old" dynamic checkers before loading them again.

[cdce8p]

Even though these get pickled, they shouldn't increase the linter object size too much as the checkers are already included in self._checkers.

[cdce8p]

Not sure if it's really necessary to reregister all messages here. The code works fine even if we don't as the re-register just overwrites it with the same info. (The code only fails if it doesn't match).

In contrast to self._checkers or the reports, the message store doesn't save a reference to the checker itself.

I've included it here for completeness but it would simplify the logic a bit if don't do it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.96%. Comparing base (3de1fbc) to head (5ec6ad9).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10642   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         176      176           
  Lines       19502    19521   +19     
=======================================
+ Hits        18715    18734   +19     
  Misses        787      787           

Files with missing lines	Coverage Δ
pylint/config/config_initialization.py	98.94% <100.00%> (+0.03%)	⬆️
pylint/lint/parallel.py	93.05% <100.00%> (+0.19%)	⬆️
pylint/lint/pylinter.py	96.30% <100.00%> (+0.07%)	⬆️
pylint/reporters/reports_handler_mix_in.py	95.34% <100.00%> (+0.34%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Pierre-Sassoulas]

I'm not sure we should add multiprocessing logic in the message store / message id store directly ? I don't understand why we do it either, as the content of the message id store should never ever change ?

Regarding the timing for 4.0, we don't have a pylint version compatible with python 3.14 and it's been out for around a week now, I'd like to release today. Maybe this can wait for 4.1.0 (and 4.1.0 can come very fast) ? ("last minute multiprocessing fix" did sound ominous but I was expecting something less complex to be honest)

[cdce8p]

I'm not sure we should add multiprocessing logic in the message store / message id store directly ? I don't understand why we do it either, as the content of the message id store should never ever change ?

Technically they should change whenever we fully remove / deregister a checker. That's just nothing we have done so far. The re-register was assumed to be save as it overwrites the entries in the message store with the same contents. Technically it works which is why we could remove the message de-register logic. Will include that in the next commits.

Regarding the timing for 4.0, we don't have a pylint version compatible with python 3.14 and it's been out for around a week now, I'd like to release today. Maybe this can wait for 4.1.0 (and 4.1.0 can come very fast) ? ("last minute multiprocessing fix" did sound ominous but I was expecting something less complex to be honest)

Will move it to 4.1.0. Yes it's truly last minute and some more testing might be good. However, from what I've seen so far it does resolve a quite annoying error not having ever warning from a custom extension listed twice. Another advantage of moving it to 4.1.0 would be that if it does accidentally break stuff, users could continue to use 4.0 for the time being.

[Pierre-Sassoulas]

Technically they should change whenever we fully remove / deregister a checker. That's just nothing we have done so far.

Why would we want to deregister a checker ? Once the configuration is read all checkers and extensions are "final", right ? I had a plan that I never opened an issue for to have a generated static base for the message id store / message store with all the messages from the known checkers / extensions, then only deal dynamically with external plugin's message (to have a better startup time, obviously). Is there something wrong with this approach ?

[cdce8p]

Technically they should change whenever we fully remove / deregister a checker. That's just nothing we have done so far.

Why would we want to deregister a checker ? Once the configuration is read all checkers and extensions are "final", right ?

We do re-register dynamic plugins though in the worker init. Currently, that just means they these checkers are added twice as the first entry is still present. That's why this PR first de-registers the "old" checker before attempting to re-register it again.

pylint/pylint/lint/parallel.py

Lines 55 to 58 in 6ce6032

	# Re-register dynamic plugins, since the pool does not have access to the
	# astroid module that existed when the linter was pickled.
	_worker_linter.load_plugin_modules(_worker_linter._dynamic_plugins, force=True)
	_worker_linter.load_plugin_configuration()

I had a plan that I never opened an issue for to have a generated static base for the message id store / message store with all the messages from the known checkers / extensions, then only deal dynamically with external plugin's message (to have a better startup time, obviously). Is there something wrong with this approach ?

Not that I can think of. The original code worked because re-registering a message with the same definition is basically a no-op. In contrast to the checker itself which get's added twice.

[jacobtylerwalls]

Thanks, I like how you avoided the need to cache the enables and disables and instead just made the re-registration more of a no-op.

[Pierre-Sassoulas]

Great second draft ! A lot simpler, and not touching the message stores anymore. Maybe we could cover more of the new code with tests ?

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 5ec6ad9