Uploaded image for project: 'OpenMRS Core'
  1. OpenMRS Core
  2. TRUNK-4487

Module architecture fails to populate aware_of modules in ModuleClassLoader if they start in the wrong order



    • Complexity:



      Currently, saying that module A is aware_of module B does not imply anything about module startup order. So it may be that module A starts before module B.

      However the code in org.openmrs.module.ModuleClassLoader#collectAwareOfModuleImports(Module) is written such that it does expect aware_of module to already be started, so if module A starts first, then its ModuleClassLoader ends up not aware of module B.

      This causes ATLAS-110 for me.

      How to reproduce

      This error depends on random ordering (because of a Hashmap), so you may have to fiddle a bit to see it.

      1. Start up OpenMRS with just the uiframework module and the atlas module
      2. Go to (host)/(webapp)/atlas/map.page → If you get a 500 status, you have experienced the error (and it should be reproducible)
      3. If you haven't experienced the error yet, add any other module and check (host)/(webapp)/atlas/map.page again (repeat step 3 with more modules until you get a 500 on that page)

      Dev Notes

      The existing code does this:

      		for (String awareOfPackage : module.getAwareOfModules()) {
      			Module awareOfModule = ModuleFactory.getModuleByPackage(awareOfPackage);
      			if (ModuleFactory.isModuleStarted(awareOfModule)) {
      				publicImportsMap.put(awareOfModule.getModuleId(), awareOfModule);

      I originally proposed in TRUNK-3995 that modules should not start before modules they are aware_of. Rowan Seymour [X] disagreed, and we ended up canceling that ticket. One solution to this ticket would be to do what I originally suggested in TRUNK-3995.

      An alternative could be to change org.openmrs.module.ModuleClassLoader#collectAwareOfModuleImports so that it checks against something like ModuleFactory.isModuleStartedOrWillStart(Module). I haven't worked out whether this will introduce other problems. (E.g. if an aware_of module doesn't start right, do we actually refresh the whole context in a way that re-creates the ModuleClassLoader.)

      FYI Burke Mamlin, Alexis Duque, this is a potential blocker for including the Atlas module in the reference application.

      Possible approaches

      1. (My preferred approach) As described in TRUNK-3995, modules should start up in order so that if module B is aware_of module A, then module A starts first. If this leads to an unresolvable cycle, the fail (in the same way we do if you have an unresolvable cycle of requires_module clauses).

      2. As described in this ticket's description under "An alternative could be...", and build on what Alexis does in his first comment (see https://github.com/alexisduque/openmrs-core/commit/e392c3f201ea2f2afcddae3aa62fc4e918c565b7 ). However we can't trust that "loaded modules" == "modules that will start" since it's possible to have a loaded-but-stopped module (I think). So you'd need to add an explicit feature to ask the module factory/framework about modules that will start. Further, you'd need to work through the code and figure out how to test this approach.


          Issue Links



              lluismf Lluis Martinez [X] (Inactive)
              darius Darius Jazayeri [X] (Inactive)
              0 Vote for this issue
              5 Start watching this issue