vdenotaris / spring-boot-security-saml-sample

SBS3 — A sample SAML 2.0 Service Provider built on Spring Boot.
https://sbs3.vdenotaris.com
Apache License 2.0
564 stars 351 forks source link

Possible thread leak #33

Closed boyanl closed 7 years ago

boyanl commented 7 years ago

There seem to be 2 cases of threads that aren't properly terminated, which causes memory leaks when the app is re-deployed, e.g. on Tomcat

In WebSecurityConfig:

Calling .cancel() + .purge() on the Timer instance, and calling MultiThreadedHttpConnectionManager.shutdownAll() when the config is destroyed seems to solve the issue. (It's not a big deal for the sample app, but I'm guessing that someone else might also use it as a template and run into the same issue)

Note: Using MultiThreadedHttpConnectionManager.shutdownAll() seems a bit suspect, as it shuts down all instances in the current JVM, but it's the only way to terminate the ReferenceQueueThread that I mentioned

vdenotaris commented 7 years ago

Hi @boyanl. You are perfectly right: I was actually already aware of this issue, but it is definitely time to fix it. I will do it within the next release.

Thanks for your input, Cheers, V.

gimerstedt commented 7 years ago

this should be a non-issue when running it as a standalone fat jar, as opposed to using an application server, correct?

boyanl commented 7 years ago

Yeah, shouldn't be an issue with a fat jar