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

Use of ApplicationEventPublisher in a reactive call stack [SPR-16481] #21025

Open
spring-projects-issues opened this issue Feb 9, 2018 · 5 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 9, 2018

Rob Winch opened SPR-16481 and commented

Currently there is no equivalent of reactive ApplicationEventPublisher which means the only way to publish events is blocking and cannot be performed in a WebFlux application. This impacts Spring Security applications which currently cannot publish authentication events. See spring-projects/spring-security#4961


Affects: 5.0.3

Issue Links:

  • #21831 Support for non-blocking event listener methods

1 votes, 8 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2018

Juergen Hoeller commented

The use of ApplicationEventPublisher isn't necessarily blocking, and in particular not dependent on I/O. Depending on the ApplicationEventMulticaster setup (e.g. SimpleApplicationEventMulticaster.setTaskExecutor), events may even be spun off to other threads immediately. But even if they're published on the same thread (as in the default setup): As long as the listeners don't perform blocking I/O, a publishEvent call shouldn't really be considered as blocking either, should it?

So I don't think we need a Reactive Streams based variant of publishEvent here. All we need is either an async ApplicationEventMulticaster setup with immediate publishEvent hand-off (which we could arrange in Boot 2.0 by simply configuring SimpleApplicationEventMulticaster.setTaskExecutor), or simply documentation that the listener implementations need to avoid blocking I/O like anything else does in a reactive stack.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2018

Rob Winch commented

The use of ApplicationEventPublisher isn't necessarily blocking

This seems like it is relying on implementation specific details

As long as the listeners don't perform blocking I/O, a publishEvent call shouldn't really be considered as blocking either, should it?

Again it seems like this would be relying on implementation specific details. If they need to do blocking IO, how would it be done? For example, a listener that writes audit events to a data store for authentication success / failure could not be used.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 9, 2018

Juergen Hoeller commented

Well, I would argue that ApplicationEventPublisher is at worst agnostic in that respect, and at best expects the listeners to not block. The whole point of event publication is quick hand-off for the publisher and no guarantees for immediate listener execution. If a listener really needs to do blocking I/O, in can either assume that it'll only be called by callers who don't care (that's an implementation detail then but by the listeners, not by the multicaster) or it'll perform the blocking tasks in an asynchronous fashion (using a TaskExecutor or even using Reactor itself). Alternatively, in order to protect against potentially misaligned listener implementations, we also allow for async hand-off right there in the ApplicationEventMulticaster setup. I believe that between those two approaches we have a proper solution available.

After all, reactive applications need some thorough setup and strict implementation rules everywhere in any case. Bean impls can't do blocking I/O in their init callbacks either for example, otherwise BeanFactory.getBean calls might block. Enforcing listener implementations to comply here sounds quite straightforward to me, much more so since we're talking about events to begin with. To be clear, I don't want to go to the extent of providing a org.reactivestreams.Publisher based variant of ApplicationEventPublisher.publishEvent: This would suggest that you can wait on listener completion... and that would be against the grain of the event model, arguably relying on implementation-specific behavior much more than the other way round. Maybe we should officially clarify that in publishEvent's javadoc.

@jhoeller
Copy link
Contributor

@jhoeller jhoeller commented Jul 8, 2019

I've finally revised the javadoc on publishEvent accordingly, suggesting that no specific semantics are implied beyond a hand-off to the multicaster - and that event listeners are encouraged to individually use asynchronous execution for longer-running and potentially blocking operations. The latter has always been possible programmatically or with the use of @Async, and is now more sophisticated in 5.2 with reactive event listener methods (#21831).

For reactive event publication purposes, the best you can do to integrate the hand-off step into a reactive pipeline is Mono.fromRunnable(() -> context.publishEvent(...)), I suppose. Since we are not propagating anything other than the event itself between an event publisher and an event listener (as per the idea of decoupled events), we are currently unable to propagate a Reactor context as well, so a publishEvent variant with a Mono return type would arguably send a misleading signal at this point.

I'm moving this to the backlog for revisiting a fully Reactor-based multicaster with tighter publisher-listener interaction at a later point. For the time being, anything that the Reactor context could contain will have to be included in the event instance itself, including contextual transaction and security holders, so that a listener can independently operate on the complete specific context that it needs.

@codependent
Copy link

@codependent codependent commented Sep 24, 2020

I found this issue after opening an ticket for failing to use a @TransactionalEventListener in a Webflux application.

At the moment it seems it can't be done. Are there any plans to provide a Reactor-based ApplicationEventMulticaster that would allow the transactional context to be propagated?

Regarding the suggested workaround:

For the time being, anything that the Reactor context could contain will have to be included in the event instance itself, including contextual transaction and security holders, so that a listener can independently operate on the complete specific context that it needs.

Could you clear up how this could be done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.