twilio / twilio-java

A Java library for communicating with the Twilio REST API and generating TwiML.
MIT License
491 stars 429 forks source link

Class loader leak in Twilio.java due to Runtime addShutdownHook() #690

Open sharamall opened 2 years ago

sharamall commented 2 years ago

The Runtime.getRuntime().addShutdownHook() links the current classloader to the jvm classloader. This causes a classloader leak in Tomcat redeployments with WARs. In case anyone is unaware, Tomcat loads applications in a separate Classloader so that the entire webapp, including its static variables and class definitions, can be reloaded if the WAR is replaced. By adding a hook to the Runtime, the anonymous Thread subclass captures the static variable executorService. This causes a reference to the containing Class instance (Twilio.class) to be referenced from the root classloader, and so it is ineligible for GC, since the root classloader is not unloaded until the JVM itself closes.

The shutdown of the executorService should not be tied to the JVM shutdown. There are other scenarios (Tomcat redeploys) where the static variables should be unloaded without the destruction of the JVM.

I don't know if there is a solution to this other than to revert pull request 502 and require that consumers of the API explicitly call destroy.

Steps to Reproduce

  1. Write an application in Maven that deploys in WAR format.
  2. Copy the war to the Tomcat webapps directory.
  3. Attach VisualVM or another profiler
  4. Deploy a new war with the same name, so that Tomcat reloads the context.
  5. Notice that the loaded classes almost doubles and the memory usage increases.
claudiachua commented 2 years ago

Hi @sharamall, Thanks for bringing this to our attention. You can either open a PR to remove the functionality and our team will review it based on our review backlog or I can add this to our internal backlog to be prioritized