enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
202 stars 34 forks source link

App initialiser is called before default repository is created #8119

Closed alansemenov closed 4 years ago

alansemenov commented 4 years ago

On a clean instance of XP (when com.enonic.cms.default is not created yet) it's important that Java initialiser in apps is called after the default repo is created. We have several apps (app-features, Headless starter) that create data via Java initialiser and if they are in "deploy" folder when clean XP is started, they will give NPE and fail to create data.

To test locally:

  1. Put this into deploy folder
  2. Stop XP
  3. Delete repo folder
  4. Start XP

image

rymsha commented 4 years ago

Note: it is not caused by #8114 because it was merged after the bug was discovered

sigdestad commented 4 years ago

Hmm, which version of XP does this apply to?

alansemenov commented 4 years ago

7.3

alansemenov commented 4 years ago

Tested on master - same thing. Good thing is that it will virtually never happen in a real environment because you would typically first start XP and then deploy apps, so it's not a blocker. Still need to fix this, I think.

rymsha commented 4 years ago

It looks like a regression from #7800 fix. Before cluster health-check was holding other services from starting up (which leads to deadlocks) but giving bootstrap initalizer a chance (about 1-2 sec) to setup a repo.

sigdestad commented 4 years ago

Cant we use startlevels?

rymsha commented 4 years ago

We already use startlevel values. They are for different cases and don't help here.

Also, note: https://bnd.bndtools.org/chapters/305-startlevels.html

A dynamic OSGi system should be ablet to startup in any order. Startlevels are often abused to hide bundles that do not handle their dynamic dependencies correctly. Hiding these bugs by controlling the start ordering is dangerous since it removes the symptom but the cause can still bite at a later inconvenient time.

https://www.aqute.biz/2017/04/24/aggregate-state.html

In OSGi there is no ordering. Get over it.

This is not the first time this bug appears. #7003 attempted to fix it, but "missed" one dependency - SystemInitializer

Unfortunately adding extra dependency would just sweep the problem under the mat.

sigdestad commented 4 years ago

I guess this is fine as long as the dependencies are properly defined, and init is part of the life cycle

rymsha commented 4 years ago

It is a regression of Content Projects implementation. It used to be ContentService which initialized default cms repo. Now it is ProjectService. ContentService has no dependency on ProjectService and they startup order is not guaranteed anyhow. We will add a bogus dependency to guarantee order.