Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConcurrentModificationException in DispatcherServlet with asynchronous ApplicationEventMulticaster [SPR-17442] #21974

Closed
spring-issuemaster opened this Issue Oct 29, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Oct 29, 2018

Jared Wiltshire opened SPR-17442 and commented

We are occasionally seeing a ConcurrentModificationException thrown from DispatcherServlet.initHandlerMappings(ApplicationContext context) due to the sorting of the handler mappings from two different threads (DispatcherServlet line 590 in 5.0.10).

I traced the cause of this back to our use of SimpleApplicationEventMulticaster with an asynchronous task executor.

What is happening is that FrameworkServlet.initWebApplicationContext() calls configureAndRefreshWebApplicationContext(cwac) which fires off a ContextRefreshedEvent. This results in onApplicationEvent(ContextRefreshedEvent event) being called from an executor thread which then calls onRefresh(event.getApplicationContext()). Further down in initWebApplicationContext() (FrameworkServlet line 557) it checks the boolean refreshEventReceived and calls onRefresh(wac) again from the main thread as there is no synchronization.

 


Affects: 5.0.10

Issue Links:

  • #21980 StandardEvaluationContext does not support concurrent variable access

Backported to: 5.0.11

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 5, 2018

Juergen Hoeller commented

I've revised this towards an AtomicBoolean onRefreshTriggered which is being explicitly checked in both places now. Whoever gets there first takes the token and is responsible for the actual onRefresh call. In particular, if the ContextRefreshedEvent happens to come after the initWebApplicationContext() phase (as in your scenario), it will simply be ignored now.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 5, 2018

Juergen Hoeller commented

Turns out it's not quite as straightforward since we support DispatcherServlet re-refreshs, as rarely as that may be used in practice...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 5, 2018

Juergen Hoeller commented

On review, full synchronization of the onRefresh callback seems to be necessary for all scenarios to work correctly, in combination with the existing refreshEventReceived flag which skips an unnecessary re-refresh on initial setup in a regular single-threaded bootstrap scenario.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 5, 2018

Jared Wiltshire commented

Thanks Juergen Hoeller for looking at this. Fix looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.