Issue #5087 Use lock to protect Deployment Mgr startup errors

[janbartel]

Closes #5087

The synchronize is there to protect the MultiException, which is not threadsafe. This is one possible solution to removing the synchronize in DeploymentManager: introduce a lock to protect the MultiException. A different solution would be to use a CopyOnWriteArrayList and then pump them all into a MultiException to throw.

[olamy]

LGTM but maybe could be easy to make MultiExceptions thread safe by using CopyOnWriteArrayList?

[lachlan-roberts]

Why do need to protect this MulitException from being used from different threads?

The only place addOnStartupError(t) is called has an isStarting() check guarding it, and the only place we check it is in doStart(). Wouldn't this all be done in the same thread.

[lorban]

LGTM, especially as this fixes a bug (onStartupErrors was not accessed from within a lock in doStart()) but I do find @lachlan-roberts 's comment pertinent.

[janbartel]

@lachlan-roberts @lorban during DeploymentManager.doStart(), it calls AppProvider.start() on all of its child app providers. When these start some of them start a new thread. These threads can start deploying apps they find immediately, at which time they call back into DeploymentManager.requestAppGoal(App, String) which collects the exceptions. Thus, as there are potentially multiple threads calling back into DeploymentManager and affecting the MultiException, it's best to protect it.