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

bugfix: fix ApplicationContext already closed problem when Seata server using ShutdownHook to destroy. #4032

Merged

Conversation

Pinocchio2018
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

in develop version,when ShutdownHook doing destroyAll,an Exception happened because ApplicationContext is already closed

14:56:22.666 ERROR --- [             ShutdownHook] i.s.core.rpc.netty.NettyServerBootstrap  : shutdown execute error:org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext@5a4c638d has been closed already
==>
java.lang.IllegalStateException: org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext@5a4c638d has been closed already
	at org.springframework.context.support.AbstractApplicationContext.assertBeanFactoryActive(AbstractApplicationContext.java:1093) ~[spring-context-5.2.14.RELEASE.jar:5.2.14.RELEASE]
	at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:1126) ~[spring-context-5.2.14.RELEASE.jar:5.2.14.RELEASE]
	at io.seata.spring.boot.autoconfigure.provider.SpringBootConfigurationProvider.get(SpringBootConfigurationProvider.java:100) ~[classes/:na]
	at io.seata.spring.boot.autoconfigure.provider.SpringBootConfigurationProvider.get(SpringBootConfigurationProvider.java:85) ~[classes/:na]
	at io.seata.spring.boot.autoconfigure.provider.SpringBootConfigurationProvider.access$200(SpringBootConfigurationProvider.java:46) ~[classes/:na]
	at io.seata.spring.boot.autoconfigure.provider.SpringBootConfigurationProvider$1.intercept(SpringBootConfigurationProvider.java:61) ~[classes/:na]
	at io.seata.config.FileConfiguration$$EnhancerByCGLIB$$862af1eb.getConfig(<generated>) ~[classes/:na]
	at io.seata.discovery.registry.nacos.NacosRegistryServiceImpl.getServiceName(NacosRegistryServiceImpl.java:214) ~[classes/:na]
	at io.seata.discovery.registry.nacos.NacosRegistryServiceImpl.unregister(NacosRegistryServiceImpl.java:92) ~[classes/:na]
	at io.seata.core.rpc.netty.NettyServerBootstrap.shutdown(NettyServerBootstrap.java:187) ~[classes/:na]
	at io.seata.core.rpc.netty.AbstractNettyRemotingServer.destroy(AbstractNettyRemotingServer.java:126) ~[classes/:na]
	at io.seata.core.rpc.ShutdownHook$DisposablePriorityWrapper.destroy(ShutdownHook.java:113) ~[classes/:na]
	at io.seata.core.rpc.ShutdownHook.destroyAll(ShutdownHook.java:82) ~[classes/:na]
	at io.seata.core.rpc.ShutdownHook.run(ShutdownHook.java:66) ~[classes/:na]
<==

Ⅱ. Does this pull request fix one issue?

fix #4028

Ⅲ. Why don't you add test cases (unit test/integration test)?

I don't know how to write one in this problem.

Ⅳ. Describe how to verify it

  1. checkout develop branch in idea,
  2. using nacos as registry center
  3. start ServerApplication in debug mode
  4. stop the seata server, check if exceptions happens

Ⅴ. Special notes for reviews

@Pinocchio2018 Pinocchio2018 force-pushed the develop_fix_server_shutdown_problem branch from eeb08ca to 79149e3 Compare September 22, 2021 07:38
@Pinocchio2018 Pinocchio2018 force-pushed the develop_fix_server_shutdown_problem branch 2 times, most recently from b079858 to e0fa703 Compare September 22, 2021 08:31
@Pinocchio2018 Pinocchio2018 changed the title bugfix: let ServerRunner do destroy instead ShutdownHook. (#4028) bugfix: fix ApplicationContext already closed problem when Seata server using ShutdownHook to destroy. Sep 22, 2021
@Pinocchio2018 Pinocchio2018 force-pushed the develop_fix_server_shutdown_problem branch from e0fa703 to e2c62ef Compare September 22, 2021 09:08
@funky-eyes funky-eyes added this to the 1.5.0 milestone Sep 22, 2021
@funky-eyes funky-eyes added first-time contributor first-time contributor module/server server module type: bug Category issues or prs related to bug. labels Sep 22, 2021
@Pinocchio2018 Pinocchio2018 force-pushed the develop_fix_server_shutdown_problem branch from e2c62ef to 6cc27fd Compare September 22, 2021 09:58
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Pinocchio2018 Pinocchio2018 force-pushed the develop_fix_server_shutdown_problem branch from 5770abd to c5ab0f8 Compare September 26, 2021 07:50
…_shutdown_problem

# Conflicts:
#	changes/1.5.0.md
#	changes/en-us/1.5.0.md
#	seata-spring-autoconfigure/seata-spring-autoconfigure-core/src/test/java/io/seata/spring/boot/autoconfigure/BasePropertiesTest.java
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2021

Codecov Report

Merging #4032 (9993202) into develop (8fd705b) will increase coverage by 0.00%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #4032   +/-   ##
==========================================
  Coverage      49.91%   49.91%           
- Complexity      3766     3767    +1     
==========================================
  Files            698      698           
  Lines          23498    23509   +11     
  Branches        2907     2910    +3     
==========================================
+ Hits           11729    11735    +6     
- Misses         10589    10593    +4     
- Partials        1180     1181    +1     
Impacted Files Coverage Δ
...c/main/java/io/seata/server/ServerApplication.java 33.33% <ø> (ø)
...er/src/main/java/io/seata/server/ServerRunner.java 57.69% <50.00%> (-8.98%) ⬇️
server/src/main/java/io/seata/server/Server.java 75.00% <100.00%> (ø)
...o/seata/server/coordinator/DefaultCoordinator.java 55.37% <0.00%> (+0.53%) ⬆️

@funky-eyes funky-eyes merged commit 8456fb6 into apache:develop Sep 26, 2021
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/server server module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

in the development version,unregister service fails
5 participants