AbstractApplicationEventMulticaster "lost" application listener [SPR-12545]

[spring-projects-issues]

Jaroslav Beran opened SPR-12545 and commented

All versions younger then 3.2.8 are affected.

There is synchronization bug.

The cache "retrieverCache" is being cleared within synchronized blocks at methods:
public void addApplicationListener(ApplicationListener listener)
public void addApplicationListenerBean(String listenerBeanName)
public void removeApplicationListener(ApplicationListener listener)
public void removeApplicationListenerBean(String listenerBeanName)
public void removeAllListeners()
The application listeners (defaultRetriever.applicationListeners and defaultRetriever.applicationListenerBeans) are modified there as well.

But at the method:
protected Collection<ApplicationListener> getApplicationListeners(ApplicationEvent event)
the access is different.
There is only one synchronized block where the copy of listeners is created.
After the synchronized block the algorithm works with the copy of list of listeners.
If the thread context is switched after the synchronized block and new listener is added than this one is never added to "retrieverCache"
and of course the new listener is not fired because the new listener is added only to list of listeners of defaultRetriever.

The situation description (in detail):

• there is the source component which periodically multicasts the event,
• some component has already registered application listener (listenerA) for the event and this one is correctly fired
  and the component receives the event (the "retrieverCache" contains at least the record for this listenerA,
• another component registers its new application listener (listenerB) at runtime, what happened:
  • the method addApplicationListener(ApplicationListener listener) is invoked, new listener (listenerB) is added and the "retrieverCache" is cleared,
• the source component multicasts the event, what happend:
  • because the "retrieverCache" is clear (this.retrieverCache.get(cacheKey) is null at line148 and the "else" branch is processed) and new record is being prepared,
    at "synchronized block" the copy of current listeners is created (there are at least two listeners for our components - listenerA and listenerB),
  • during the "synchronized block" another component (another thread) is registering its new listener (listenerC)
    and the method addApplicationListener(ApplicationListener listener) is blocked because of "synchonized",
  • when the "synchronized block" (at "else" branch) is left, thread context is switched and new listenerC is added and cache is cleared, when the thread context is switched back the algorithm works with old list of listeners and the new one (listenerC) is not included to the copy of listeners ("retrieverCache" is still empty),
    at the end of the algorithm the "retriever" contains only two listeners listenerA and listenerB, BUT NOT listenerC.
    The "retriever" is put to "retrieverCache" and if the source component multicasts the event the listenerC IS NOT fired.

Solution (or guick fix):

• remaining algorithm must be included to synchronized block (the getApplicationListeners(ApplicationEvent event) method snippet):

  if (retriever != null) {
  return retriever.getApplicationListeners();
  }
  else {
  retriever = new ListenerRetriever(true);
  LinkedList<ApplicationListener> allListeners = new LinkedList<ApplicationListener>();
  Set<ApplicationListener> listeners;
  Set<String> listenerBeans;
  synchronized (this.defaultRetriever) {
  listeners = new LinkedHashSet<ApplicationListener>(this.defaultRetriever.applicationListeners);
  listenerBeans = new LinkedHashSet<String>(this.defaultRetriever.applicationListenerBeans);

    for (ApplicationListener listener : listeners) {
      if (supportsEvent(listener, eventType, sourceType)) {
        retriever.applicationListeners.add(listener);
        allListeners.add(listener);
      }
    }
    if (!listenerBeans.isEmpty()) {
      BeanFactory beanFactory = getBeanFactory();
      for (String listenerBeanName : listenerBeans) {
        ApplicationListener listener = beanFactory.getBean(listenerBeanName, ApplicationListener.class);
        if (!allListeners.contains(listener) && supportsEvent(listener, eventType, sourceType)) {
          retriever.applicationListenerBeans.add(listenerBeanName);
          allListeners.add(listener);
        }
      }
    }
    OrderComparator.sort(allListeners);
    if (this.beanClassLoader == null ||
        (ClassUtils.isCacheSafe(eventType, this.beanClassLoader) &&
            (sourceType == null || ClassUtils.isCacheSafe(sourceType, this.beanClassLoader)))) {
      this.retrieverCache.put(cacheKey, retriever);
    }
  }
  return allListeners;
  

  }

---

Affects: 3.2.9, 4.1.3

Issue Links:

• Deadlock publishing event while creating listener bean [SPR-12739] #17336 Deadlock publishing event while creating listener bean

Backported to: 4.0.9, 3.2.13

[spring-projects-issues]

Juergen Hoeller commented

Good catch! AbstractApplicationEventMulticaster populates its ListenerRetriever cache in a fully synchronized fashion now, just skipping the full synchronization if it won't cache a retriever to begin with.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

This is available in recent 4.1.4 snapshots now, scheduled for release next week... Would be great if you could give it an early try!

Juergen