From 39600b901f25b545b57a66129848b1641215de2e Mon Sep 17 00:00:00 2001 From: Roman Matiushchenko Date: Sun, 29 Sep 2019 20:23:05 +0300 Subject: [PATCH] Dispose default Scheduler AbstractUserDetailsReactiveAuthenticationManager creates parallel Scheduler with daemon=false Threads. It is recommended to dispose such Schedulers to be able exit the VM Fixes gh-7492 --- ...rDetailsReactiveAuthenticationManager.java | 16 ++++++++-- ...ailsServiceAuthenticationManagerTests.java | 31 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java b/core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java index 6ad3f35b53..a38a9dafbc 100644 --- a/core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java +++ b/core/src/main/java/org/springframework/security/authentication/AbstractUserDetailsReactiveAuthenticationManager.java @@ -18,6 +18,7 @@ package org.springframework.security.authentication; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.factory.DisposableBean; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; @@ -45,7 +46,7 @@ import org.springframework.util.Assert; * @author EddĂș MelĂ©ndez * @since 5.2 */ -public abstract class AbstractUserDetailsReactiveAuthenticationManager implements ReactiveAuthenticationManager { +public abstract class AbstractUserDetailsReactiveAuthenticationManager implements ReactiveAuthenticationManager, DisposableBean { protected final Log logger = LogFactory.getLog(getClass()); @@ -55,7 +56,8 @@ public abstract class AbstractUserDetailsReactiveAuthenticationManager implement private ReactiveUserDetailsPasswordService userDetailsPasswordService; - private Scheduler scheduler = Schedulers.newParallel("password-encoder"); + Scheduler scheduler = Schedulers.newParallel("password-encoder"); + private boolean defaultScheduler = true; private UserDetailsChecker preAuthenticationChecks = user -> { if (!user.isAccountNonLocked()) { @@ -138,6 +140,10 @@ public abstract class AbstractUserDetailsReactiveAuthenticationManager implement */ public void setScheduler(Scheduler scheduler) { Assert.notNull(scheduler, "scheduler cannot be null"); + if (this.defaultScheduler) { + this.defaultScheduler = false; + this.scheduler.dispose(); + } this.scheduler = scheduler; } @@ -171,4 +177,10 @@ public abstract class AbstractUserDetailsReactiveAuthenticationManager implement */ protected abstract Mono retrieveUser(String username); + @Override + public void destroy() { + if (this.defaultScheduler) { + this.scheduler.dispose(); + } + } } diff --git a/core/src/test/java/org/springframework/security/authentication/ReactiveUserDetailsServiceAuthenticationManagerTests.java b/core/src/test/java/org/springframework/security/authentication/ReactiveUserDetailsServiceAuthenticationManagerTests.java index 73011471b0..faf36d6b0b 100644 --- a/core/src/test/java/org/springframework/security/authentication/ReactiveUserDetailsServiceAuthenticationManagerTests.java +++ b/core/src/test/java/org/springframework/security/authentication/ReactiveUserDetailsServiceAuthenticationManagerTests.java @@ -33,6 +33,8 @@ import org.springframework.security.core.userdetails.ReactiveUserDetailsService; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.crypto.password.PasswordEncoder; import reactor.core.publisher.Mono; +import reactor.core.scheduler.Scheduler; +import reactor.core.scheduler.Schedulers; import reactor.test.StepVerifier; /** @@ -136,4 +138,33 @@ public class ReactiveUserDetailsServiceAuthenticationManagerTests { .expectError(BadCredentialsException.class) .verify(); } + + @Test + public void destroyWhenDefaultSchedulerThenShouldDispose() { + assertThat(manager.scheduler.isDisposed()).isFalse(); + manager.destroy(); + assertThat(manager.scheduler.isDisposed()) + .as("default Scheduler should be disposed") + .isTrue(); + } + + @Test + public void destroyWhenCustomSchedulerThenShouldNotDispose() { + manager.setScheduler(Schedulers.parallel()); + manager.destroy(); + assertThat(manager.scheduler.isDisposed()) + .as("custom Scheduler should not be disposed") + .isFalse(); + } + + @Test + public void setSchedulerWhenSetCustomSchedulerThenDisposeDefault() { + Scheduler defaultScheduler = manager.scheduler; + assertThat(defaultScheduler.isDisposed()).isFalse(); + manager.setScheduler(Schedulers.parallel()); + assertThat(defaultScheduler.isDisposed()) + .as("default Scheduler should be disposed") + .isTrue(); + } + }