Background
With platform 3, we have an opportunity to address some of the otherwise unaddressable issues with OpenMRS Core. The big technical flaw that is not solveable in Platform 1 or Platform 2 is TRUNK-5344, i.e., issues that arise when two modules try to load different copies of the same class (previous Talk thread). While the flaw is real, it’s a result of the way ClassLoaders work in OpenMRS currently: each module has a ModuleClassLoader where the module’s classes and libraries are loaded, but shared libraries are almost always preferentially loaded from the OpenmrsClassLoader, which delegates to the first copy of any given class in either the base OpenMRS classpath or the first ModuleClassLoader that has a copy of that class (TRUNK-5344 usually triggers in the more narrow case where a module initially loads a class as part of it’s activation, and consequently on the ModuleClassLoader, but later references it in the global context, and so winds up with a copy of the class from a different ModuleClassLoader, resulting in the LinakgeError reported there).
There’s a relatively straight-forward and correct fix for this (ensuring ModuleClassLoaders are isolated from one another unless module A depends on module B either by declaring that it requires module B or that it’s aware of module B). However, implementing this fix is likely to break at least some modules which are implicitly depending on a dependency from an external module without realizing it (normally this would be caused by using a library that’s available on the build and test classpaths but not bundled as part of the OMOD).
Simultaneously, I’d like to get us out of owning too much ClassLoader separation code. This kind of code is always complicated, hard to reason about and hard to maintain.
There are also two core pieces of OpenMRS (the Spring Context and the Hibernate SessionFactory) that do wind-up needing a “global” view of all available beans or JPA entities in order to function properly; it’s these requirements that seemed to lead to the development of the OpenmrsClassLoader as an escape hatch.
In the past, we’ve talked about solving this either with OSGi (which would likely require massive changes to both OpenMRS Core and OpenMRS modules) or (what I’ve been advocating for) jboss-modules (docs), which is a modular class-loading system used, among other places, to implement Keycloak’s plugin system (which shares some of our concerns).
Proposed Solution
I’ve (finally) had a chance to spike on this and I think it’s possible to leverage JBoss Modules to resolve TRUNK-5344, allow different modules to depend on different versions of the same library while more or less retaining the way we use Spring and Hibernate. Effectively, we’d be remove ModuleClassLoaders and replacing them with JBoss Modules classloaders with a little customized loader to support modules that a module is only aware-of. Of course, nothing is free, so this has the following trade-offs:
- Adding or removing a module will require a restart (this also means we’d probably lose hot-reloading of O2 UIs)
- Dropping support for
org.openmrs.module.Extensions and extension-points (this will require some re-working of the Legacy UI) - Classes in modules will be preferentially loaded from the module’s own classes and libraries and secondarily from any dependencies. Undeclare dependencies will hard-fail at runtime.
- It’s possible that fully realizing this will mean killing the current iteration of the installation wizard.
I also still need to test this idea a lot more thoroughly; I’ve currently gotten it to work with some very bare-bones but illustrative modules, but no real-world modules. However, I do think that this can mostly be handled by changes only to the platform internals and not, e.g., to the module format or the module descriptor format.
If this sounds like a good direction to go in, I’ll continue working on my spike and get it to a PR that we can discuss in more detail.