eclipse-ee4j / glassfish

Eclipse GlassFish
https://eclipse-ee4j.github.io/glassfish/
369 stars 136 forks source link

Fixes resources leak in `WebappClassLoader` #24941

Open avpinchuk opened 1 month ago

avpinchuk commented 1 month ago

When we undeploy a web application that uses deploy time libraries, these libraries remain open until the domain stops/restart.

Now we disable deploy time libraries caching and close their associated class loaders during undeploy.

dmatej commented 3 weeks ago

But then what is the purpose of those libraries? If they could be shared between web applications, now the class loader will be created again for every war, right? Then they can be included directly to the war file right?

Then I understand why the test failed on windows - we don't have any way to unload these libraries. Perhaps it could be done automatically when we don't have any application classloader referring their classloader. Do I understand it well now? The last time I was reviewing on the cell phone, sorry ;-)

avpinchuk commented 3 weeks ago

But then what is the purpose of those libraries?

E.g. in failed tests ServerAuthModule installed with --libraries option.

If they could be shared between web applications, now the class loader will be created again for every war, right? Then they can be included directly to the war file right?

Before their ClassFinders shared, but we should deploy every app with the --libraries option. Those libraries not visible by default to others app. Now every app has its own ClassFinder

Then I understand why the test failed on windows - we don't have any way to unload these libraries. Perhaps it could be done automatically when we don't have any application classloader referring their classloader. Do I understand it well now?

You understand correctly ) But they are never released automatically, even if we undeploy an app.

This service has singleton scope:

https://github.com/eclipse-ee4j/glassfish/blob/f256659a33a8444aefd6e8ffb7d9dec1b369d6f8/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppLibClassLoaderServiceImpl.java#L53

This map

https://github.com/eclipse-ee4j/glassfish/blob/f256659a33a8444aefd6e8ffb7d9dec1b369d6f8/nucleus/core/kernel/src/main/java/com/sun/enterprise/v3/server/AppLibClassLoaderServiceImpl.java#L67

never cleaned and contains URLClassLoaders and we never invoke their close method. Thus, app lib classloaders stay open.

avpinchuk commented 3 weeks ago

I'm working on another option now. Stateless class loader which doesn't store open JAR file but open and close it on demand when we need lo load a class. This approach fixes errors in all places, where app libs used, not only in web app.