diff --git a/lib/galaxy/queue_worker/__init__.py b/lib/galaxy/queue_worker/__init__.py index e71ca5f7c03..a998c012f00 100644 --- a/lib/galaxy/queue_worker/__init__.py +++ b/lib/galaxy/queue_worker/__init__.py @@ -286,33 +286,40 @@ def _get_new_toolbox(app: "UniverseApplication", save_integrated_tool_panel: boo """ Generate a new toolbox, by constructing a toolbox from the config files, and then adding pre-existing data managers from the old toolbox to the new toolbox. + + Serialized under ``app._toolbox_lock`` against ``remove_tool_by_id`` and + ``invalidate_index_cache``: a reload queued by an earlier conf write (the + install that precedes an uninstall) otherwise interleaves its rebuild — + including a potentially long inline index populate — with the removal, + and the swapped-in toolbox resurrects the just-removed tool. """ tool_configs = app.config.tool_configs - new_toolbox: ToolBox - if getattr(app.config, "use_lazy_toolbox", False) and getattr(app, "tool_source_store", None) is not None: - new_toolbox = LazyToolBox( - config_filenames=tool_configs, - tool_root_dir=app.config.tool_path, - app=app, - tool_source_store=app.tool_source_store, - cache_size=getattr(app.config, "lazy_toolbox_cache_size", 500), - save_integrated_tool_panel=save_integrated_tool_panel, - ) - else: - new_toolbox = ToolBox( - tool_configs, - app.config.tool_path, - app, - save_integrated_tool_panel=save_integrated_tool_panel, - ) - new_toolbox.data_manager_tools = app.toolbox.data_manager_tools - app.datatypes_registry.load_datatype_converters(new_toolbox, use_cached=True) - app.datatypes_registry.load_external_metadata_tool(new_toolbox) - load_lib_tools(new_toolbox) - for tool in new_toolbox.data_manager_tools.values(): - new_toolbox.register_tool(tool) - app._toolbox = new_toolbox + with app._toolbox_lock: + new_toolbox: ToolBox + if getattr(app.config, "use_lazy_toolbox", False) and getattr(app, "tool_source_store", None) is not None: + new_toolbox = LazyToolBox( + config_filenames=tool_configs, + tool_root_dir=app.config.tool_path, + app=app, + tool_source_store=app.tool_source_store, + cache_size=getattr(app.config, "lazy_toolbox_cache_size", 500), + save_integrated_tool_panel=save_integrated_tool_panel, + ) + else: + new_toolbox = ToolBox( + tool_configs, + app.config.tool_path, + app, + save_integrated_tool_panel=save_integrated_tool_panel, + ) + new_toolbox.data_manager_tools = app.toolbox.data_manager_tools + app.datatypes_registry.load_datatype_converters(new_toolbox, use_cached=True) + app.datatypes_registry.load_external_metadata_tool(new_toolbox) + load_lib_tools(new_toolbox) + for tool in new_toolbox.data_manager_tools.values(): + new_toolbox.register_tool(tool) + app._toolbox = new_toolbox def reload_data_managers(app, **kwargs): diff --git a/lib/galaxy/tools/lazy_toolbox.py b/lib/galaxy/tools/lazy_toolbox.py index 0d47da823f0..437669c37d1 100644 --- a/lib/galaxy/tools/lazy_toolbox.py +++ b/lib/galaxy/tools/lazy_toolbox.py @@ -1774,6 +1774,16 @@ class LazyToolBox(ToolBox): send_control_task(self.app, "reload_tool_source_cache", noop_self=True) except Exception as e: log.warning("Broadcasting index removal of %s raised: %s", tool_id, e) + # A toolbox reload queued before this removal (the conf write of + # the preceding install) may already have swapped a NEW toolbox + # into ``app.toolbox``, built from the index as it stood before + # ``remove_index_entry`` above — with the tool still registered. + # ``self`` is then the superseded instance and cleaning it alone + # leaves ``app.toolbox`` serving the uninstalled tool. The swap + # holds the same lock, so the current object is stable here. + current = getattr(self.app, "toolbox", None) + if current is not self and isinstance(current, LazyToolBox): + current._remove_tool_in_memory(tool_id, remove_from_panel=remove_from_panel) return result def _remove_tool_in_memory(self, tool_id: str, remove_from_panel: bool = True): diff --git a/test/unit/app/tools/test_lazy_tool.py b/test/unit/app/tools/test_lazy_tool.py index e0fbce9cb9d..e6114d7fba2 100644 --- a/test/unit/app/tools/test_lazy_tool.py +++ b/test/unit/app/tools/test_lazy_tool.py @@ -564,6 +564,25 @@ def test_remove_tool_by_id_broadcasts_reload_to_peers(monkeypatch): assert "doomed" not in box._tools_by_id +def test_remove_tool_by_id_also_cleans_swapped_in_toolbox(monkeypatch): + # A reload queued before the removal can swap a new toolbox into + # app.toolbox, rebuilt from the pre-removal index — cleaning only + # ``self`` would leave the new box serving the uninstalled tool. + old_box = _registry_box() + new_box = _registry_box() + entry = _entry(id="doomed") + for box in (old_box, new_box): + box._tool_index = ToolIndex() + box._tool_index.add_entry(entry) + box._register_lazy_entry(entry) + old_box.app.toolbox = new_box + monkeypatch.setattr(queue_worker_mod, "send_control_task", lambda app, task, **kwargs: None) + old_box.remove_tool_by_id("doomed") + assert "doomed" not in old_box._tools_by_id + assert "doomed" not in new_box._tools_by_id + assert "tool_doomed" not in new_box._tool_panel + + def test_tool_file_on_disk_answers_from_index(tmp_path): box = _seam_box() box._index_source_paths_cache = None