Skip to content

Future<Void> method with @Gateway causes thread leak #3635

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

Closed
toyopilgrim opened this issue Sep 22, 2021 · 3 comments · Fixed by #3899
Closed

Future<Void> method with @Gateway causes thread leak #3635

toyopilgrim opened this issue Sep 22, 2021 · 3 comments · Fixed by #3899

Comments

@toyopilgrim
Copy link

In what version(s) of Spring Integration are you seeing this issue?

org.springframework.boot:spring-boot-starter-integration: '2.2.6.RELEASE'

Describe the bug

If response type is Future<Void>, the method with @Gateway causes thread leak. New threads are constantly spawned without reusing or disposing. It was confirmed on thread dump I collected in my application. Even though each memory usage is quite small, threads being racked up will eventually lead OOM.
I assume that this is happening because of no acknowledgement with Void.

However, there's a solution. It simply can be avoided by replacing it with void send along with ExecutorService in order to keep it asynchronous. (Non-blocking messaging is what I wanna do)
e.g.

@Bean
public MessageChannel sampleChannel() {
    return new PublishSubscribeChannel(Executors.newCachedThreadPool());
}

This is maybe a bug or should be described clearly in the docs if not a bug imhp.

To Reproduce

  • Register bean at a config class
@Bean
public MessageChannel sampleChannel() {
    return MessageChannels.publishSubscribe().get();
}
  • Make MessagingGateway have a method returns Future as following
@MessagingGateway
public interface SampleMessagingGateway { 
  @Gateway(requestChannel = "sampleChannel")  
  Future<Void> send(Payload payload);
}
  • send(payload) at a producer class.

  • Have a consumer class which contains method like

@ServiceActivator(inputChannel = "sampleChannel")
public void onSample(Payload payload){
  // do something 
}

As a result, threads spawned by that consumers stays in the lifetime of application apparently unless the application is restarted or so. Thus this will eventually lead OOM.

Expected behavior

  • It won't cause thread leak with Future<Void>

Or

  • Described in doc well. This behavior seems not described explicitly at asyn-gateway doc. If you wanna deal with it void type, the solution using ExecutorService should be recommended.

Sample

See above.

Thx in advance.

@toyopilgrim toyopilgrim added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Sep 22, 2021
@artembilan artembilan added in: core and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Sep 22, 2021
@artembilan
Copy link
Member

See similar discussion about Mono<Void> on Gitter: https://gitter.im/spring-projects/spring-integration?at=614aef15f428f97a9f9d0077.

I think this is that part which is missed from the logic and therefore it is not described in the docs.

At a glance it does not make much sense to have a Future<Void>, but if you build an async pipe-line and just rely on the completion of the Future as a fact, it really could be handy since even void services could be called from executors.
See more explanation here: https://stackoverflow.com/questions/50567120/what-is-futurevoid

@artembilan artembilan added this to the 6.0.x milestone Sep 22, 2021
@artembilan
Copy link
Member

For now I schedule this for the next version since we never advertised support for Future<Void>, so this could be treated as a new feature, but we always can change our mind and with enough justification back-port the fix to the current 5.5.x generation.

@artembilan artembilan self-assigned this Sep 30, 2022
@artembilan artembilan modified the milestones: 6.0.x, 6.0.0-RC1 Sep 30, 2022
@artembilan
Copy link
Member

Looks like Kotlin suspend functions without return type will suffer from the same problem.

Will try to fix this shortly.

artembilan added a commit to artembilan/spring-integration that referenced this issue Sep 30, 2022
Fixes spring-projects#3635

When `Future<Void>` & `Mono<Void>` is used as a messaging gateway return type,
the application hangs out on this barrier which may lead to the out of memory eventually

* Add support for the `Future<Void>` & `Mono<Void>` messaging gateway return type
and ensure an asynchronous call for the `gateway.send(Message)` operation and
its exception handling.
In case of successful call, the `Future` is fulfilled with `null` and `Mono` is completed as empty
artembilan added a commit to artembilan/spring-integration that referenced this issue Oct 3, 2022
Fixes spring-projects#3635

When `Future<Void>` & `Mono<Void>` is used as a messaging gateway return type,
the application hangs out on this barrier which may lead to the out of memory eventually

* Add support for the `Future<Void>` & `Mono<Void>` messaging gateway return type
and ensure an asynchronous call for the `gateway.send(Message)` operation and
its exception handling.
In case of successful call, the `Future` is fulfilled with `null` and `Mono` is completed as empty
garyrussell pushed a commit that referenced this issue Oct 4, 2022
* GH-3635: Add Future<Void> & Mono<Void> to gateway

Fixes #3635

When `Future<Void>` & `Mono<Void>` is used as a messaging gateway return type,
the application hangs out on this barrier which may lead to the out of memory eventually

* Add support for the `Future<Void>` & `Mono<Void>` messaging gateway return type
and ensure an asynchronous call for the `gateway.send(Message)` operation and
its exception handling.
In case of successful call, the `Future` is fulfilled with `null` and `Mono` is completed as empty

* * Check for `void.class` as well in the `GatewayProxyFactoryBean.isVoidReturnType`

* * Allow `Future<Void>` as a reply type of the gateway request-reply operation

* * Fix  Checkstyle violations

* * Resolve `System.err.println()` in the test code

* Add `Thread.currentThread.interrupt()` to the `InterruptedException` block in the test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants