Skip to content

AbstractUserDetailsReactiveAuthenticationManager default Scheduler should be disposed #7492

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
robotmrv opened this issue Sep 29, 2019 · 4 comments · Fixed by #7493
Closed
Assignees
Labels
in: core An issue in spring-security-core type: bug A general bug
Milestone

Comments

@robotmrv
Copy link
Contributor

Summary

AbstractUserDetailsReactiveAuthenticationManager creates new parallel Scheduler which creates Threads with daemon=false.

Scheduler scheduler = Schedulers.newParallel("password-encoder");

As states Schedulers.newParallel javadoc

daemon – false if the Scheduler requires an explicit Scheduler.dispose() to exit the VM.

Actual Behavior

default Scheduler is not disposed

  1. on AbstractUserDetailsReactiveAuthenticationManager bean destruction
  2. or if custom Scheduler was set

Expected Behavior

default Scheduler should be disposed

  1. on AbstractUserDetailsReactiveAuthenticationManager bean destruction
  2. or if custom Scheduler was set

Version

5.2.0.BUILD-SNAPSHOT

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 29, 2019
@rwinch rwinch closed this as completed in 39600b9 Sep 30, 2019
rwinch added a commit that referenced this issue Sep 30, 2019
@rwinch rwinch added in: core An issue in spring-security-core type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 30, 2019
@rwinch rwinch self-assigned this Sep 30, 2019
@rwinch rwinch added this to the 5.2.0 milestone Sep 30, 2019
@rwinch rwinch reopened this Sep 30, 2019
@rwinch rwinch closed this as completed in b29106e Sep 30, 2019
@rwinch
Copy link
Member

rwinch commented Sep 30, 2019

@robotmrv I went ahead and changed this fix to use a daemon thread instead. We don't need the thread to block shutting down the JVM.

@robotmrv
Copy link
Contributor Author

@rwinch
I thought about this solution (cb5c58e#r35174748)
Schedulers.newParallel("password-encoder", Schedulers.DEFAULT_POOL_SIZE, true)
but there is another problem
if user wants to refresh application context, default scheduler will not be disposed so unused redundant Threads will be present.
So for this case it should be disposed in anyway

@rwinch
Copy link
Member

rwinch commented Sep 30, 2019

Thanks. Then perhaps it should be a static member variable w/ the default Scheduler?

@robotmrv
Copy link
Contributor Author

robotmrv commented Oct 1, 2019

yes, static default Scheduler will solve that problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core An issue in spring-security-core type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants