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 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.

        Gliffy Diagrams


            Issue Links



                lluismf Lluis Martinez
                darius Darius Jazayeri
                0 Vote for this issue
                5 Start watching this issue