Uploaded image for project: 'RESTEasy'
  1. RESTEasy
  2. RESTEASY-3596

Contextual scheduled executor service causes inadvertent Thread + classloader leak

XMLWordPrintable

    • Icon: Enhancement Enhancement
    • Resolution: Unresolved
    • Icon: Major Major
    • 6.2.15.Final, 7.0.1.Final
    • 6.2.12.Final
    • Core
    • None

      Continuing saga from
      https://issues.redhat.com/browse/RESTEASY-3314

      Thank you for the initial fix - reusing the executor very much reduces the cost of allocating a new thread for each timeout, which makes our service use much less resources.

      Unfortunately, the new implementation comes with a price. If you are not using JNDI container provided executors, then a GlobalContextualScheduledExecutorService is created for you. The only condition that closes this service is the "resteasy-shutdown" runtime shutdown hook. There is a close() method but as far as I can tell, this is never called on the global instance, and it is not clear when it would be appropriate to do so.

       

      This means if you create a new ClassLoader with a Resteasy deployment inside, run your service lifecycle, and then shut down the inner application but not the outer JVM, you end up leaking the still-running scheduled executor service thread pool. Until the runtime exit hook runs, those threads continue to run, and their context class loader is the inner-application class loader, so you end up with a memory leak.

       

      I appreciate our use case is a bit strange, with each webapp loading its own resteasy, and the fix is not necessarily easy.

      I prototyped a quick fix that may or may not be the right approach to fixing this. Instead of having an always-running executor service, create a shared java.util.Timer instance on the dispatcher. Then when we set a timeout, create a contextual runnable with the timeout action and schedule it on this shared Timer.

      This means that the running Timer thread is part of java.base rather than any app code, and as long as we are careful to not keep the context class loader and leak it ourselves, it would not prevent GC of the ClassLoader and the rest of the app.

      I have my current hacked-up fix here that we are testing: https://github.com/resteasy/resteasy/compare/6.2...stevenschlansker:Resteasy:scs-virtual-scheduler?expand=1

       

      Note, this probably also affects the GlobalContextualExecutorService, but this seems to only be used by the ResteasySeInstance so we don't have problems with it. Thanks for your consideration.

              jperkins-rhn James Perkins
              stevenschlansker Steven Schlansker (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated: