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

ApplicationFailedEvent should always quit application but jvm is kept alive by kafka consumers #2055

Closed
tnormani opened this issue Dec 21, 2021 · 4 comments · Fixed by #2060

Comments

@tnormani
Copy link

Hello,

I'm facing an issue on Spring Boot 2.5.4 under the following conditions:

  • spring-boot-starter-parent:2.5.4
  • spring-boot-starter-web:2.5.4
  • spring-kafka:2.7.6
  • embedded Tomcat won't start because Port 8080 is already in use

In this scenario an ApplicationFailedEvent is triggered and main thread is killed, but the application won't quit because kafka consumers threads are still alive. This results in a compromised application that appears like is up and running, but it is not.
I have also made a test using legacy spring-boot-starter-parent:2.2.10.RELEASE and related dependencies. In this scenario the behaviour is different and the Spring boot application quits as expected.

I've published a simple project to reproduce the problem at this link https://github.com/tnormani/application-failed-event-testcase .
Project requires port 8080 to be already bound by something else and a kafka broker for the consumer connection.
In the project you will find also a ApplicationFailedListener I created during tests, but it's not referenced in the spring.factories file to make test case behave as expected.

I've also raised a question on StackOverflow (https://stackoverflow.com/questions/70378200/how-to-make-a-spring-boot-application-quit-on-tomcat-failure) days ago but after the comparison with spring-boot-starter-parent:2.2.10.RELEASE and some further testing, I thought it would be better if you could have a look at it.
Thanks in advance.

@garyrussell
Copy link
Contributor

This is really a question for the Spring Boot folks, not here, since you are describing a possible change in Boot's behavior.

From this project's perspective, the consumers will be stopped and closed when the application context is closed.

@wilkinsona
Copy link
Member

wilkinsona commented Jan 4, 2022

While Boot's behaviour has changed, I believe Spring Kafka would suffer the same problem with any SmartLifecycle that runs after KafkaListenerEndpointRegistry and fails to start. There is a bit more reasoning in spring-projects/spring-boot#29147, but the TL;DR is that I think the registry needs to clean itself up when destroy() is called without a prior call to stop() as Framework doesn't guarantee that stop() will be called when context refresh fails.

@wilkinsona
Copy link
Member

I believe Spring Kafka would suffer the same problem with any SmartLifecycle that runs after KafkaListenerEndpointRegistry and fails to start.

I just verified this theory and it is the case. I took the sample app, replaced the dependency on spring-boot-starter-web was one on spring-boot-starter and added the following SmartLifecycle:

@Component
static class FaultyLifecycle implements SmartLifecycle {
    
    private volatile boolean running = false;

    @Override
    public void start() {
        System.out.println(this + " start");
        this.running = true;
        throw new RuntimeException();
    }

    @Override
    public void stop() {
        this.running = false;
        System.out.println(this + " stop");
        
    }

    @Override
    public boolean isRunning() {
        return this.running;
    }

    @Override
    public int getPhase() {
        return Integer.MAX_VALUE - 1;
    }
    
}

The JVM does not exit after the startup failure due to the non-daemon thread named org.springframework.kafka.KafkaListenerEndpointContainer#0-0-C-1 that's still alive.

@garyrussell
Copy link
Contributor

Thanks, Andy; I checked the other endpoint registries (JMS, RabbitMQ) and the containers there all implement DisposableBean. I missed that when I ported the code to Kafka.

@garyrussell garyrussell self-assigned this Jan 4, 2022
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jan 4, 2022
Resolves spring-projects#2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jan 4, 2022
Resolves spring-projects#2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jan 4, 2022
Resolves spring-projects#2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**
garyrussell added a commit to garyrussell/spring-kafka that referenced this issue Jan 4, 2022
Resolves spring-projects#2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**
artembilan pushed a commit that referenced this issue Jan 4, 2022
Resolves #2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**
artembilan pushed a commit that referenced this issue Jan 4, 2022
Resolves #2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/MessageListenerContainer.java
artembilan pushed a commit that referenced this issue Jan 4, 2022
Resolves #2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/MessageListenerContainer.java

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/config/KafkaListenerEndpointRegistry.java
#	spring-kafka/src/main/java/org/springframework/kafka/listener/MessageListenerContainer.java
artembilan pushed a commit that referenced this issue Jan 4, 2022
Resolves #2055

If context initialization fails, `Lifecycle.stop()` is not called.
Containers must be stopped from `DisposableBean` in this case.

**cherry-pick to 2.7.x, 2.6.x, 2.5.x**

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/listener/MessageListenerContainer.java

# Conflicts:
#	spring-kafka/src/main/java/org/springframework/kafka/config/KafkaListenerEndpointRegistry.java
#	spring-kafka/src/main/java/org/springframework/kafka/listener/MessageListenerContainer.java
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.

3 participants