spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.42k stars 38.07k forks source link

Reject @EventListener asynchronous method that have a return type [SPR-14113] #18685

Closed spring-projects-issues closed 8 years ago

spring-projects-issues commented 8 years ago

Stéphane Nicoll opened SPR-14113 and commented

See https://spring.io/blog/2015/02/11/better-application-events-in-spring-framework-4-2#comment-2605353079

The underlying infrastructure does not get the reply of the method since it is asynchronous so even though the method replies something ultimately, we can send the reply). I guess one way would be to wrap the actual method invocation in its own async processor of some kind.


Referenced from: commits https://github.com/spring-projects/spring-framework/commit/bee1b77af59a7a23e364510764a74aa3593144f9

spring-projects-issues commented 8 years ago

Stéphane Nicoll commented

I added a failing test here.

If the async method returns a Future or CompletableFuture we could get a handle to something that would supply the value and reuse the infrastructure to process that reply asynchronously. But that would force the user to actually change the method signature once async is in place. We could easily warn the user if the return typer is non-void and @Async is present. How would we determine the Executor to use to process those replies? Ideally we would just reuse the one that the method has declared (or the default) but I don't see an easy way to get a handle to that.

My best guess at this point would be to inject AsyncExecutionAspectSupport and invoke it with a custom MethodInvocation that would further process the return type. It feels to me that this could be a interesting as an internal framework feature. Juergen Hoeller what do you think?

spring-projects-issues commented 8 years ago

Stéphane Nicoll commented

There is no easy way for us to collect the reply of such method without blocking the caller thread. Given that async method are not meant to return a value anyway, we've decided to reject this use case and an exception is now thrown with such arrangement.

spring-projects-issues commented 8 years ago

Stéphane Nicoll commented

Unfortunately this change brings a (quite obvious now) package tangle. I'll revert this for the time being, we still have time to figure out what we want to do for 4.3 GA

spring-projects-issues commented 8 years ago

Stéphane Nicoll commented

Let's rather not do that check. There is no clear way to avoid the cycle and that cycle is actually a hint that we shouldn't do it.