SpringBoot ApplicationAvailabilityBean is not thread-safe

ApplicationAvailabilityBean#onApplicationEvent could be invoked concurrently, we should replace HashMap(event map) with ConcurrentHashMap.

Details in jdk - HashMap.java - L89

Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally.

Comment From: mhalbritter

In which case would this method be invoked concurrently? The JavaDoc from EventListener states that concurrency is used if the method is annotated with @Async, which isn't the case here.

Comment From: dugenkui03

In which case would this method be invoked concurrently?

spring boot allow developer inject ApplicationAvailabilityBean in multiple custom beans and publish their custom AvailabilityState event. there is a chance that ApplicationAvailabilityBean#onApplicationEvent could be invoked concurrently.

Comment From: mhalbritter

It's not expected that users inject the bean and call onApplicationEvent directly. If you want to change the application availability status, you should emit an AvailabilityChangeEvent. The spring event system will then ensure that these listeners are called. And they are not called concurrently.

Comment From: wilkinsona

I think we should accept this. While onApplicationEvent is being called on one thread by the event multicaster and it mutates the map, another thread could call one of the get… methods that reads from the map.

Comment From: mhalbritter

Ah, yeah, you're right. I missed that case. Will merge the PR.

Comment From: dugenkui03

... they are not called concurrently.

@mhalbritter I trace the code from AvailabilityChangeEvent, and find there is nothing to ensure that listeners are not called concurrently —— I can't find any lock or things like BlockingQueue.

// 1. multiple thread invoke this static method concurrently and publish different state.
 AvailabilityChangeEvent.publish(this.eventPublisher, ex, LivenessState.BROKEN);

// 2.  call multicastEvent in ‘AbstractApplicationContext#publishEvent(Object, ResolvableType)‘
//      still don't have any lock, 
getApplicationEventMulticaster().multicastEvent(applicationEvent, eventType)

// 3. multiple thread could invoke 'SimpleApplicationEventMulticaster#multicastEvent' at same time and publish different event concurrently.
protected void invokeListener(ApplicationListener<?> listener, ApplicationEvent event) {
                 ...
        doInvokeListener(listener, event);
}

private void doInvokeListener(ApplicationListener listener, ApplicationEvent event) {
    try {
        listener.onApplicationEvent(event);
    }
        ...
}