diff --git a/src/backend/InvenTree/plugin/registry.py b/src/backend/InvenTree/plugin/registry.py index 8f8a450180..bb321ec697 100644 --- a/src/backend/InvenTree/plugin/registry.py +++ b/src/backend/InvenTree/plugin/registry.py @@ -9,6 +9,7 @@ import importlib import importlib.util import os import sys +import threading import time from collections import OrderedDict from importlib.machinery import SourceFileLoader @@ -91,6 +92,9 @@ class PluginsRegistry: # before the mismatch is reported as an error (and reloading for that hash stops) MAX_MISMATCH_RELOADS = 2 + # Maximum number of times a reload is repeated, if further reloads are requested while loading + MAX_PENDING_RELOADS = 3 + # This list of plugins are *always* enabled, and are loaded by default # This is because they provide core functionality to the InvenTree system # Other 'builtin' plugins are automatically loaded, but can be disabled by the user @@ -140,6 +144,8 @@ class PluginsRegistry: self.errors: dict[str, list[Any]] = {} # Holds errors discovered during loading self.loading_lock = Lock() # Lock to prevent multiple loading at the same time + self._loading_thread: Optional[int] = None # Thread currently holding the lock + self._reload_pending: bool = False # Reload requested while lock was held # flags self.plugins_loaded = ( @@ -491,15 +497,86 @@ class PluginsRegistry: Returns: bool: True if the registry was reloaded, False if the reload was skipped or failed """ - # Do not reload when currently loading - if self.is_loading: - logger.debug('Skipping reload - plugin registry is currently loading') - return False - if not self.loading_lock.acquire(blocking=False): - logger.exception('Could not acquire lock for reload_plugins') + if self._loading_thread != threading.get_ident(): + # Another thread is currently reloading the registry. + # Request that it reloads again once complete - otherwise any state + # change made in the meantime (e.g. activating a plugin) would be lost, + # as the registry hash would no longer reflect the database state. + self._reload_pending = True + logger.info( + 'Plugin registry is currently loading - deferring reload request' + ) + else: + logger.debug('Skipping reload - plugin registry is currently loading') + return False + result = False + + try: + self._loading_thread = threading.get_ident() + + for attempt in range(self.MAX_PENDING_RELOADS + 1): + self._reload_pending = False + + if attempt > 0: + logger.info( + 'Plugin registry state changed during reload - reloading again' + ) + + # Discard any PluginConfig objects cached during the previous pass + self._clear_plugin_config_cache() + + # A deferred request may have required a full reload + full_reload = force_reload = collect = True + + result = self._reload_plugins( + full_reload=full_reload, + force_reload=force_reload, + collect=collect, + clear_errors=clear_errors and attempt == 0, + update_hash=update_hash, + _internal=_internal, + ) + + if not self._reload_pending: + break + else: + logger.warning( + 'Plugin registry reload requested repeatedly during reload - giving up' + ) + self._reload_pending = False + finally: + self._loading_thread = None + self.loading_lock.release() + + if self._reload_pending: + # A request arrived between the final check and releasing the lock + return self.reload_plugins( + full_reload=True, + force_reload=True, + collect=True, + update_hash=update_hash, + _internal=_internal, + ) + + return result + + def _clear_plugin_config_cache(self) -> None: + """Discard the per-request cache of PluginConfig objects (see get_plugin_configs).""" + InvenTree.cache.set_session_cache(f'plugin_configs:{self.registry_hash}', None) + + def _reload_plugins( + self, + full_reload: bool, + force_reload: bool, + collect: bool, + clear_errors: bool, + update_hash: bool, + _internal: Optional[list], + ) -> bool: + """Perform the plugin registry reload - the caller must hold the loading lock.""" # Reset the loading error state if clear_errors: self.errors = {} @@ -563,10 +640,6 @@ class PluginsRegistry: log_error('reload_plugins', scope='plugins') return False - finally: - # Ensure the lock is released always - self.loading_lock.release() - return True def plugin_dirs(self) -> list[str]: diff --git a/src/backend/InvenTree/plugin/test_plugin.py b/src/backend/InvenTree/plugin/test_plugin.py index f96f0ec260..9ff53c4cdf 100644 --- a/src/backend/InvenTree/plugin/test_plugin.py +++ b/src/backend/InvenTree/plugin/test_plugin.py @@ -736,6 +736,58 @@ class RegistryTests(TestQueryMixin, PluginRegistryMixin, TestCase): self.assertIsNone(registry.mismatch_hash) self.assertEqual(registry.mismatch_count, 0) + def test_reload_requested_during_reload(self): + """Test that a reload requested by another thread while loading is not lost. + + Previously, a reload requested while another thread held the loading lock + was silently dropped. If a plugin was activated in that window, the registry + (and its hash) would not reflect the new state until the server restarted. + """ + import threading + + from plugin.models import PluginConfig + + slug = 'sample-printer-machine-plugin' + + PluginConfig.objects.filter(key=slug).update(active=False) + registry.reload_plugins(full_reload=True, collect=True, force_reload=True) + self.assertNotIn(slug, [p.slug for p in registry.with_mixin('machine')]) + + original_load = registry._load_plugins + calls = [] + + def load_plugins(*args, **kwargs): + """Activate the plugin from 'another thread' during the first reload.""" + calls.append(1) + + if len(calls) == 1: + PluginConfig.objects.filter(key=slug).update(active=True) + + # The reload request from the other thread cannot acquire the lock + results = [] + thread = threading.Thread( + target=lambda: results.append( + registry.reload_plugins(full_reload=True, collect=True) + ) + ) + thread.start() + thread.join() + self.assertEqual(results, [False]) + + return original_load(*args, **kwargs) + + with patch.object(registry, '_load_plugins', side_effect=load_plugins): + self.assertTrue( + registry.reload_plugins( + full_reload=True, collect=True, force_reload=True + ) + ) + + # The deferred request caused a second reload pass + self.assertEqual(len(calls), 2) + self.assertFalse(registry._reload_pending) + self.assertIn(slug, [p.slug for p in registry.with_mixin('machine')]) + def test_registry_hash_order_independence(self): """Test that the registry hash does not depend on plugin iteration order.