ApplicationContext.refresh() causes stale listeners to be added to ApplicationEventMulticaster

[eknidev]

The problem is caused by the change in Spring AbstractApplicationContext, in particular how it handles the addition of ApplicationListener objects.
In short, all of the registered ApplicationListener objects are retained even after application context is refreshed,
at which point the stale listeners become unusable as AbstractLifecycleBean(s) would have had their destroy methods invoked.

The change that caused this problem:
#21858
Commit:
c8c0737

With this commit all listeners are added to AbstractApplicationContext's list applicationListeners instead of just adding them to an instance of ApplicationEventMulticaster.
This causes stale listeners to be added to a new instance of ApplicationEventMulticaster when the AbstractApplicationContext is refreshed via its refresh() method.
The refresh() method creates a new ApplicationEventMulticaster but then it adds all of the stale listeners (by registerListeners()) from its list of listeners.
Subsequently, when the refresh finishes (AbstractApplicationContext.finishRefresh()), a new instance of ApplicationEventMulticaster publishes the refresh event too all registered listeners, including the stale listeners whose destroy methods were called as part of the refresh.

[jhoeller]

@wilkinsona since this one addresses a side effect of #21858 which you requested, could you please double-check that the revision (in master and 5.1.x at this point) still works for your purposes, exposing all application listeners that you intend to see from getApplicationListeners()? I'll backport it to 5.0.x then where we backported the original change as well.

[wilkinsona]

This looks good to me with 5.1.5 snapshots. The sample provided with spring-projects/spring-boot#14490 stops as expected.

[jhoeller]

Alright, good to hear. Thanks for the immediate feedback!

[jhoeller]

@eknizat since this is available in 5.1.5.BUILD-SNAPSHOT from https://repo.spring.io/snapshot now, do you have a chance to give it an early try in your scenario before the official 5.1.5 release on Wednesday?

[eknidev]

The 5.1.5 snapshot works as expected. Thanks.

[platonovr]

This issue is closed, but @jhoeller can you check this line, please condition

I have the following problem: on the application startup there is a call of prepareRefresh(), so it works like that

this.earlyApplicationListeners = new LinkedHashSet<>(this.applicationListeners);

There are this.applicationListeners is an empty collection.
Then it's needed to refresh the Application Context, so it provokes the refresh() at AbstractApplicationContext, then goes to prepareRefresh and then checks

this.earlyApplicationListeners == null,

but it's an empty LinkedHashSet, not null, so then it comes to
this.applicationListeners.clear();
and all the listeners are removed, but this collection is not empty, so I'm loosing all of applicationListeners there.
Just FYI it was filled by CXF transports here is the hack

All in all, is there a reason to add isEmpty checking too? Or what will be the better decision?
Thank you in advance.