From 855412a8d12a3da9c61fb2794222bffdba28fc45 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sun, 15 Feb 2026 12:27:19 -0800 Subject: [PATCH 1/6] Enforce that getWebPartFactories() is called after upgrade is complete --- api/src/org/labkey/api/module/DefaultModule.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/src/org/labkey/api/module/DefaultModule.java b/api/src/org/labkey/api/module/DefaultModule.java index 2e9c6a7956d..50e663e1e58 100644 --- a/api/src/org/labkey/api/module/DefaultModule.java +++ b/api/src/org/labkey/api/module/DefaultModule.java @@ -353,6 +353,11 @@ public void afterUpdate(ModuleContext moduleContext) { if (null == _webPartFactories) { + // We promise that this method is not called until upgrade is complete. Enforce that to (for example) + // prevent memory leaks associated with modules removed due to missing dependencies. + if (ModuleLoader.getInstance().isUpgradeRequired()) + throw new IllegalStateException("getWebPartFactories() is being called before upgrade is complete"); + Collection wpf = new ArrayList<>(); // Get all the Java webpart factories From 748bf46250cd36c3a5d534f26baf9249e4097162 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 16 Feb 2026 07:54:04 -0800 Subject: [PATCH 2/6] Prune modules with missing dependencies before core upgrade to prevent memory leaks --- .../org/labkey/api/module/DefaultModule.java | 8 -- api/src/org/labkey/api/module/Module.java | 7 +- .../org/labkey/api/module/ModuleLoader.java | 79 ++++++++++--------- 3 files changed, 43 insertions(+), 51 deletions(-) diff --git a/api/src/org/labkey/api/module/DefaultModule.java b/api/src/org/labkey/api/module/DefaultModule.java index 50e663e1e58..489e7b7ea20 100644 --- a/api/src/org/labkey/api/module/DefaultModule.java +++ b/api/src/org/labkey/api/module/DefaultModule.java @@ -341,9 +341,6 @@ public void afterUpdate(ModuleContext moduleContext) { } - // TODO: Move getWebPartFactories() and _webPartFactories into Portal... shouldn't be the module's responsibility - // This should also allow moving SimpleWebPartFactoryCache and dependencies into Internal - private final Object FACTORY_LOCK = new Object(); @Override @@ -353,11 +350,6 @@ public void afterUpdate(ModuleContext moduleContext) { if (null == _webPartFactories) { - // We promise that this method is not called until upgrade is complete. Enforce that to (for example) - // prevent memory leaks associated with modules removed due to missing dependencies. - if (ModuleLoader.getInstance().isUpgradeRequired()) - throw new IllegalStateException("getWebPartFactories() is being called before upgrade is complete"); - Collection wpf = new ArrayList<>(); // Get all the Java webpart factories diff --git a/api/src/org/labkey/api/module/Module.java b/api/src/org/labkey/api/module/Module.java index 51221c02a6a..071ba3d393f 100644 --- a/api/src/org/labkey/api/module/Module.java +++ b/api/src/org/labkey/api/module/Module.java @@ -146,7 +146,7 @@ default boolean canBeEnabled(Container c) /** License name: e.g. "Apache 2.0", "LabKey Software License" */ @Nullable String getLicense(); - /** License URL: e.g. "http://www.apache.org/licenses/LICENSE-2.0" */ + /** License URL: e.g. "https://www.apache.org/licenses/LICENSE-2.0" */ @Nullable String getLicenseUrl(); /** @@ -182,7 +182,7 @@ default void startBackgroundThreads() /** * The application is shutting down "gracefully". Module - * should do any cleanup (file handles etc) that is required. + * should do any cleanup (file handles, etc.) that is required. * Note: There is no guarantee that this will run if the server * process is without a nice shutdown. */ @@ -190,8 +190,7 @@ default void startBackgroundThreads() /** * Return Collection of WebPartFactory objects for this module. - * NOTE: This may be called before startup, but will never be called - * before upgrade is complete. + * NOTE: This may be called early, before init() and startup(), but after core upgrade. * * @return Collection of WebPartFactory (empty collection if none) */ diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index b487ce09903..65f6871e113 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -22,7 +22,6 @@ import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; -import org.apache.xmlbeans.XmlBeans; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.Constants; @@ -428,7 +427,7 @@ public void updateModuleDirectory(File dir, File archive) { throw new IllegalStateException("Not a valid module: " + archive.getName()); } - moduleCreated = moduleList.get(0); + moduleCreated = moduleList.getFirst(); if (null != archive) moduleCreated.setZippedPath(archive); @@ -480,7 +479,7 @@ public void updateModuleDirectory(File dir, File archive) // avoid error in startup, DefaultModule does not expect to see module with same name initialized again ((DefaultModule) moduleCreated).unregister(); _moduleFailures.remove(moduleCreated.getName()); - pruneModules(moduleList); + ensureModulesSupportLabKeyDatabase(moduleList); initializeModules(moduleList); Throwable t = _moduleFailures.get(moduleCreated.getName()); @@ -576,7 +575,7 @@ private void doInit(Execution execution) throws ServletException // set the project source root before calling .initialize() on modules var modules = getModules(); - Module coreModule = modules.isEmpty() ? null : modules.get(0); + Module coreModule = modules.isEmpty() ? null : modules.getFirst(); if (coreModule == null || !DefaultModule.CORE_MODULE_NAME.equals(coreModule.getName())) throw new IllegalStateException("Core module was not first or could not find the Core module. Ensure that Tomcat user can create directories under the /modules directory."); setProjectRoot(coreModule); @@ -622,7 +621,9 @@ private void doInit(Execution execution) throws ServletException // Prune modules before upgrading core module, see Issue 42150 synchronized (_modulesLock) { - pruneModules(_modules); + // _modules is in dependency order + ensureModulesSupportLabKeyDatabase(_modules); + verifyDependencies(_modules); } if (getTableInfoModules().getTableType() == DatabaseTableType.NOT_IN_DB) @@ -678,7 +679,6 @@ private void doInit(Execution execution) throws ServletException synchronized (_modulesLock) { checkForRenamedModules(); - // use _modules here because this List<> needs to be modifiable initializeModules(_modules); } @@ -920,21 +920,18 @@ private boolean isDevelopmentBuild(Module module) } /** - * Enumerates all the modules, removing the ones that don't support the core database + * Enumerates all the modules, removing the ones that don't support the primary database */ - private void pruneModules(List modules) + private void ensureModulesSupportLabKeyDatabase(List modules) { - Module core = getCoreModule(); - - SupportedDatabase coreType = SupportedDatabase.get(CoreSchema.getInstance().getSqlDialect()); - // Need to enumerate a copy of the list to avoid ConcurrentModificationException + SqlDialect dialect = DbScope.getLabKeyScope().getSqlDialect(); + SupportedDatabase primaryType = SupportedDatabase.get(dialect); + // Enumerate a copy of the list to avoid ConcurrentModificationException for (Module module : new ArrayList<>(modules)) { - if (module == core) - continue; - if (!module.getSupportedDatabasesSet().contains(coreType)) + if (!module.getSupportedDatabasesSet().contains(primaryType)) { - var e = new DatabaseNotSupportedException("This module does not support " + CoreSchema.getInstance().getSqlDialect().getProductName()); + var e = new DatabaseNotSupportedException("This module does not support " + dialect.getProductName()); // In production mode, treat these exceptions as a module initialization error // In dev mode, make them warnings so devs can easily switch databases removeModule(modules, module, !AppProps.getInstance().isDevMode(), e); @@ -942,6 +939,26 @@ private void pruneModules(List modules) } } + /** + * Remove modules if any of their module dependencies are missing + */ + private void verifyDependencies(List modules) + { + for (Module module : modules) + { + try + { + verifyDependencies(module); + } + catch (ModuleDependencyException e) + { + // In production mode, treat module dependency exceptions as errors + // In dev mode, make them warnings so devs can easily switch databases + removeModule(modules, module, !AppProps.getInstance().isDevMode(), e); + } + } + } + /** * Enumerates all remaining modules, initializing them and removing any that fail to initialize */ @@ -950,14 +967,10 @@ private void initializeModules(List modules) Module core = getCoreModule(); /* - * NOTE: Module.initialize() really should not ask for resources from _other_ modules, - * as they may have not initialized themselves yet. However, we did not enforce that - * so this cross-module behavior may have crept in. - * - * To help mitigate this a little, we remove modules that do not support this DB type - * before calling initialize(). - * - * NOTE: see FolderTypeManager.get().registerFolderType() for an example of enforcing this + * NOTE: Module.initialize() really should not ask for resources from _other_ modules, as they may have not + * initialized themselves yet. However, we did not enforce that so this cross-module behavior may have crept + * in. To help mitigate this a little, we previously removed modules based on supported DB type and missing + * dependencies. See FolderTypeManager.registerFolderType() for an example of enforcing this */ //initialize each module in turn @@ -968,21 +981,9 @@ private void initializeModules(List modules) try { - try - { - // Make sure all its dependencies initialized successfully - verifyDependencies(module); - module.initialize(); - } - catch (DatabaseNotSupportedException | ModuleDependencyException e) - { - // In production mode, treat these exceptions as a module initialization error - if (!AppProps.getInstance().isDevMode()) - throw e; - - // In dev mode, make them warnings so devs can easily switch databases - removeModule(modules, module, false, e); - } + // Make sure all its dependencies initialized successfully + verifyDependencies(module); + module.initialize(); } catch (Throwable t) { From 2c63180778977cfb4e8a80bf1ebfcfc782918e37 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Mon, 16 Feb 2026 12:15:04 -0800 Subject: [PATCH 3/6] Update comment --- api/src/org/labkey/api/module/ModuleLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 65f6871e113..716da793b47 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -996,7 +996,7 @@ private void initializeModules(List modules) initControllerToModule(); } - // Check a module's dependencies and throw on the first one that's not present (i.e., it was removed because its initialize() failed) + // Check a module's dependencies and throw on the first one that's not present (i.e., dependency is not present or was removed because its initialize() failed) private void verifyDependencies(Module module) { synchronized (_modulesLock) From 79db8d082cb45b6ad166c89d3ee64bc95c86dee1 Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 17 Feb 2026 10:40:01 -0800 Subject: [PATCH 4/6] More accurate and consistent method naming --- .../org/labkey/api/module/ModuleLoader.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 716da793b47..9c7b2989363 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -479,8 +479,8 @@ public void updateModuleDirectory(File dir, File archive) // avoid error in startup, DefaultModule does not expect to see module with same name initialized again ((DefaultModule) moduleCreated).unregister(); _moduleFailures.remove(moduleCreated.getName()); - ensureModulesSupportLabKeyDatabase(moduleList); - initializeModules(moduleList); + pruneModulesForDatabaseSupport(moduleList); + initializeAndPruneModules(moduleList); Throwable t = _moduleFailures.get(moduleCreated.getName()); if (null != t) @@ -622,8 +622,8 @@ private void doInit(Execution execution) throws ServletException synchronized (_modulesLock) { // _modules is in dependency order - ensureModulesSupportLabKeyDatabase(_modules); - verifyDependencies(_modules); + pruneModulesForDatabaseSupport(_modules); + pruneModulesForDependencies(_modules); } if (getTableInfoModules().getTableType() == DatabaseTableType.NOT_IN_DB) @@ -679,7 +679,7 @@ private void doInit(Execution execution) throws ServletException synchronized (_modulesLock) { checkForRenamedModules(); - initializeModules(_modules); + initializeAndPruneModules(_modules); } if (!_duplicateModuleErrors.isEmpty()) @@ -922,7 +922,7 @@ private boolean isDevelopmentBuild(Module module) /** * Enumerates all the modules, removing the ones that don't support the primary database */ - private void ensureModulesSupportLabKeyDatabase(List modules) + private void pruneModulesForDatabaseSupport(List modules) { SqlDialect dialect = DbScope.getLabKeyScope().getSqlDialect(); SupportedDatabase primaryType = SupportedDatabase.get(dialect); @@ -942,13 +942,13 @@ private void ensureModulesSupportLabKeyDatabase(List modules) /** * Remove modules if any of their module dependencies are missing */ - private void verifyDependencies(List modules) + private void pruneModulesForDependencies(List modules) { for (Module module : modules) { try { - verifyDependencies(module); + pruneModuleForDependencies(module); } catch (ModuleDependencyException e) { @@ -962,7 +962,7 @@ private void verifyDependencies(List modules) /** * Enumerates all remaining modules, initializing them and removing any that fail to initialize */ - private void initializeModules(List modules) + private void initializeAndPruneModules(List modules) { Module core = getCoreModule(); @@ -982,7 +982,7 @@ private void initializeModules(List modules) try { // Make sure all its dependencies initialized successfully - verifyDependencies(module); + pruneModuleForDependencies(module); module.initialize(); } catch (Throwable t) @@ -996,8 +996,8 @@ private void initializeModules(List modules) initControllerToModule(); } - // Check a module's dependencies and throw on the first one that's not present (i.e., dependency is not present or was removed because its initialize() failed) - private void verifyDependencies(Module module) + // Check a single module's dependencies and throw on the first one that's not present (i.e., dependency is not present or its init() threw) + private void pruneModuleForDependencies(Module module) { synchronized (_modulesLock) { From f0ba4182ea823f81ff268410e981719f9f6d9efc Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 17 Feb 2026 15:37:05 -0800 Subject: [PATCH 5/6] Clarify some logging of removed modules --- .../org/labkey/api/module/ModuleLoader.java | 33 ++++++++++++------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/api/src/org/labkey/api/module/ModuleLoader.java b/api/src/org/labkey/api/module/ModuleLoader.java index 9c7b2989363..a43af596db6 100644 --- a/api/src/org/labkey/api/module/ModuleLoader.java +++ b/api/src/org/labkey/api/module/ModuleLoader.java @@ -934,7 +934,7 @@ private void pruneModulesForDatabaseSupport(List modules) var e = new DatabaseNotSupportedException("This module does not support " + dialect.getProductName()); // In production mode, treat these exceptions as a module initialization error // In dev mode, make them warnings so devs can easily switch databases - removeModule(modules, module, !AppProps.getInstance().isDevMode(), e); + removeModule(modules, module, !AppProps.getInstance().isDevMode(), e, "load"); } } } @@ -948,13 +948,13 @@ private void pruneModulesForDependencies(List modules) { try { - pruneModuleForDependencies(module); + pruneModuleForDependencies(module, "load"); } catch (ModuleDependencyException e) { // In production mode, treat module dependency exceptions as errors // In dev mode, make them warnings so devs can easily switch databases - removeModule(modules, module, !AppProps.getInstance().isDevMode(), e); + removeModule(modules, module, !AppProps.getInstance().isDevMode(), e, "load"); } } } @@ -982,12 +982,21 @@ private void initializeAndPruneModules(List modules) try { // Make sure all its dependencies initialized successfully - pruneModuleForDependencies(module); + pruneModuleForDependencies(module, "initialize"); + } + catch (ModuleDependencyException mde) + { + removeModule(modules, module, !AppProps.getInstance().isDevMode(), mde, "load"); + continue; + } + + try + { module.initialize(); } catch (Throwable t) { - removeModule(modules, module, true, t); + removeModule(modules, module, true, t, "initialize"); } } @@ -997,37 +1006,37 @@ private void initializeAndPruneModules(List modules) } // Check a single module's dependencies and throw on the first one that's not present (i.e., dependency is not present or its init() threw) - private void pruneModuleForDependencies(Module module) + private void pruneModuleForDependencies(Module module, String action) { synchronized (_modulesLock) { for (String dependency : module.getModuleDependenciesAsSet()) if (!_moduleMap.containsKey(dependency)) - throw new ModuleDependencyException(dependency); + throw new ModuleDependencyException(dependency, action); } } private static class ModuleDependencyException extends ConfigurationException { - public ModuleDependencyException(String dependencyName) + public ModuleDependencyException(String dependencyName, String action) { - super("This module depends on the \"" + dependencyName + "\" module, which failed to initialize"); + super("This module depends on the \"" + dependencyName + "\" module, which failed to " + action); } } - private void removeModule(List modules, Module current, boolean treatAsError, Throwable t) + private void removeModule(List modules, Module current, boolean treatAsError, Throwable t, String action) { String name = current.getName(); if (treatAsError) { - _log.error("Unable to initialize module {}", name, t); + _log.error("Unable to {} module {}", action, name, t); //noinspection ThrowableResultOfMethodCallIgnored _moduleFailures.put(name, t); } else { - _log.warn("Unable to initialize module {} due to: {}", name, t.getMessage()); + _log.warn("Unable to {} module {} due to: {}", action, name, t.getMessage()); } synchronized (_modulesLock) From 6d40cccd1220fd74b029bf6149a366330feabfad Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Tue, 17 Feb 2026 20:36:02 -0800 Subject: [PATCH 6/6] Fix weird code --- .../org/labkey/api/exp/OntologyManager.java | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/api/src/org/labkey/api/exp/OntologyManager.java b/api/src/org/labkey/api/exp/OntologyManager.java index 5297e1ff8e6..209905bbc8f 100644 --- a/api/src/org/labkey/api/exp/OntologyManager.java +++ b/api/src/org/labkey/api/exp/OntologyManager.java @@ -171,25 +171,20 @@ public static DomainDescriptor fetchDomainDescriptorFromDB(String uriOrName, Con proj = c; String sql = " SELECT * FROM " + getTinfoDomainDescriptor() + " WHERE " + (isName ? "Name" : "DomainURI") + " = ? AND Project IN (?,?) "; - List ddArray = new SqlSelector(getExpSchema(), sql, uriOrName, + List ddList = new SqlSelector(getExpSchema(), sql, uriOrName, proj, ContainerManager.getSharedContainer().getId()).getArrayList(DomainDescriptor.class); - DomainDescriptor dd = null; - if (!ddArray.isEmpty()) - { - dd = ddArray.getFirst(); - // if someone has explicitly inserted a descriptor with the same URI as an existing one , - // and one of the two is in the shared project, use the project-level descriptor. - if (ddArray.size() > 1) - { - _log.debug("Multiple DomainDescriptors found for " + uriOrName); - // TODO: This check and assignment look wrong. We always return the first element. - if (dd.getProject().equals(ContainerManager.getSharedContainer())) - dd = ddArray.getFirst(); - } + if (ddList.size() > 1) + { + // if there are multiple descriptors with the same URI, prefer the first one that's not in the shared project + _log.debug("Multiple DomainDescriptors found for {}", uriOrName); + for (DomainDescriptor dd : ddList) + if (!ContainerManager.getSharedContainer().equals(dd.getProject())) + return dd; } - return dd; + + return ddList.isEmpty() ? null : ddList.getFirst(); } private static final BlockingCache DOMAIN_DESC_BY_ID_CACHE = DatabaseCache.get(getExpSchema().getScope(),2000, CacheManager.UNLIMITED,"Domain descriptors by ID", new DomainDescriptorLoader());