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

[type: fix] fix client shutdown close method call twice #4867

Merged
merged 4 commits into from
Jul 18, 2023

Conversation

847850277
Copy link
Contributor

The client's close will be called twice when it is closed.
Because of Spring When the bean is destroyed, it will proactively call the close or shutdown methods. At the same time, Shenyu also defined ShenyuClientShutdownHook. Therefore, renaming the close method to closeRepository solves the problem of being called twice.

image

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

Merging #4867 (1a03aa5) into master (b31d4a3) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #4867      +/-   ##
============================================
- Coverage     64.50%   64.49%   -0.02%     
- Complexity     8220     8225       +5     
============================================
  Files          1161     1161              
  Lines         34153    34153              
  Branches       3061     3061              
============================================
- Hits          22030    22026       -4     
- Misses        10367    10371       +4     
  Partials       1756     1756              
Impacted Files Coverage Δ
...client/core/shutdown/ShenyuClientShutdownHook.java 0.00% <0.00%> (ø)
...ster/client/etcd/EtcdClientRegisterRepository.java 97.72% <ø> (ø)
...ster/client/http/HttpClientRegisterRepository.java 0.00% <ø> (ø)
...er/client/nacos/NacosClientRegisterRepository.java 100.00% <ø> (ø)
...t/zookeeper/ZookeeperClientRegisterRepository.java 94.59% <ø> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@yu199195 yu199195 added this to the 2.6.0 milestone Jul 18, 2023
@yu199195
Copy link
Member

yu199195 commented Jul 18, 2023

client shutdown close method call twice ? how to fixed?

@847850277
Copy link
Contributor Author

client shutdown close method call twice ? how to fixed?

yes。
1、ShenyuClientRegisterRepository bean destry the close methd will be call.
2、and Runtime.getRuntime().addShutdownHook(new Thread(repository::closeRepository, name)); will be call.

By default, JavaConfig configured beans are used. If there is a close or shutdown method, this method will be automatically executed when the bean is destroyed. If you do not want to execute this method, add @ Bean (destroyMethod="") to prevent the destruction method from starting

i try rename close methd to closeRepository can solve spring call default close method.

@loongs-zhang
Copy link
Member

client shutdown close method call twice ? how to fixed?

@Target({ElementType.METHOD, ElementType.ANNOTATION_TYPE})
@Retention(RetentionPolicy.RUNTIME)
@Documented
public @interface Bean {

	//......

	/**
	 * The optional name of a method to call on the bean instance upon closing the
	 * application context, for example a {@code close()} method on a JDBC
	 * {@code DataSource} implementation, or a Hibernate {@code SessionFactory} object.
	 * The method must have no arguments but may throw any exception.
	 * <p>As a convenience to the user, the container will attempt to infer a destroy
	 * method against an object returned from the {@code @Bean} method. For example, given
	 * an {@code @Bean} method returning an Apache Commons DBCP {@code BasicDataSource},
	 * the container will notice the {@code close()} method available on that object and
	 * automatically register it as the {@code destroyMethod}. This 'destroy method
	 * inference' is currently limited to detecting only public, no-arg methods named
	 * 'close' or 'shutdown'. The method may be declared at any level of the inheritance
	 * hierarchy and will be detected regardless of the return type of the {@code @Bean}
	 * method (i.e., detection occurs reflectively against the bean instance itself at
	 * creation time).
	 * <p>To disable destroy method inference for a particular {@code @Bean}, specify an
	 * empty string as the value, e.g. {@code @Bean(destroyMethod="")}. Note that the
	 * {@link org.springframework.beans.factory.DisposableBean} callback interface will
	 * nevertheless get detected and the corresponding destroy method invoked: In other
	 * words, {@code destroyMethod=""} only affects custom close/shutdown methods and
	 * {@link java.io.Closeable}/{@link java.lang.AutoCloseable} declared close methods.
	 * <p>Note: Only invoked on beans whose lifecycle is under the full control of the
	 * factory, which is always the case for singletons but not guaranteed for any
	 * other scope.
	 * @see org.springframework.beans.factory.DisposableBean
	 * @see org.springframework.context.ConfigurableApplicationContext#close()
	 */
	String destroyMethod() default AbstractBeanDefinition.INFER_METHOD;
}

The close in ShenyuClientRegisterRepository will be called by spring, so just change the close method name.

loongs-zhang
loongs-zhang previously approved these changes Jul 18, 2023
@loongs-zhang loongs-zhang merged commit 594447e into apache:master Jul 18, 2023
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants