ultraq / thymeleaf-layout-dialect

A dialect for Thymeleaf that lets you build layouts and reusable templates in order to improve code reuse
https://ultraq.github.io/thymeleaf-layout-dialect/
Apache License 2.0
700 stars 112 forks source link

Possible Memory Leak for 2.5.3 #215

Closed Airforce111 closed 3 years ago

Airforce111 commented 3 years ago

Hi. We'd encountered a possible memory leak related to our usage of this library indirectly via groovy, on two separate projects of ours.

Java 11.0.9.1 JVM OpenJDK 64-Bit Spring Boot 2.4.3 Spring Boot Starter Thymeleaf 2.4.3 Thymeleaf Layout Dialect 2.5.3

We'd noticed increasing metaspace usage, which are not being garbage collected. This occurs after an undeploy/redeploy of the application. The ParallelWebappClassLoader, with all its loaded contents remain after each undeploy. This in turn bloats the g1 old gen and metaspace.

We'd looked at the GC Roots and they all point to a strong reference from JNI Global. Tracing shows its related to org.codehaus.groovy.reflection.ClassInfo. We don't use groovy in our projects and identified thymeleaf-layout-dialect. We decided to remove the thymeleaf-layout-dialect package to test and once removed, the problem is no longer present.

Do you perhaps have any insight as to what is going on? Is it a possible memory leak within thymeleaf layout's usage of groovy somewhere, or an issue with groovy? Is there something we can call on context destruction to clear the reference? Anybody else with similar reports?

Thanks for looking into this.

Class loaders (highlighted) that have been destroyed but is lingering due to strong references. image

The merged GC Roots. image

The only configuration for thymeleaf within our project is: image image

ultraq commented 3 years ago

Hey there, thanks for bringing this to my attention.

Question: have you tried using version 2.5.2 of the layout dialect? 2.5.3 was released only very recently, and the only change in it was an upgrade of the Groovy dependency (release notes: https://github.com/ultraq/thymeleaf-layout-dialect/releases/tag/2.5.3). If you tried it and didn't experience the memory leak, then it'd help narrow things down.

I'll probably have a check through the Groovy tickets to see if anything else like this has been raised with it, but no this is the first memory-leak-related issue I've had in a while. The last one was many years ago, all the way back with version 2.0.0 and was because of how I used a feature of Groovy, which I have since stopped doing 🙂

ultraq commented 3 years ago

Notes to self:

From an initial browse through the Groovy Jira, I stumbled across this old though unresolved issue which seems to hit many of the same keywords in the report above: https://issues.apache.org/jira/browse/GROOVY-8199 Links to many other Jiras, though many closed hinting that it has been resolved. Might be nice to read through them, get an insight into what it was about to see if it's relevant here.

Airforce111 commented 3 years ago

Question: have you tried using version 2.5.2 of the layout dialect? 2.5.3 was released only very recently, and the only change in it was an upgrade of the Groovy dependency (release notes: https://github.com/ultraq/thymeleaf-layout-dialect/releases/tag/2.5.3). If you tried it and didn't experience the memory leak, then it'd help narrow things down.

Unfortunately, we'd had the issue for a few weeks now (prior to 2.5.3). I'd only recently was able to pinpoint the issue to this package. Perhaps I can skim through your linked issues as well. Otherwise, we're at a loss of why it's occurring.

ultraq commented 3 years ago

Yes, feel free to check out that link on the Groovy Jira. The thing that stood out to me about it was the mention of a special system property, groovy.use.classvalue, introduced as part of a linked issue talking about how ClassValue causes memory leaks (https://issues.apache.org/jira/browse/GROOVY-7591). When looking at your screenshots above, there's java.lang.ClassValue in that chain of objects holding on to the memory, so it looks suspicious.

It looks like you can toggle the use of java.lang.ClassValue in Groovy by adding -Dgroovy.use.classvalue=true or -Dgroovy.use.classvalue=false to the startup parameters. That is a super old issue though, and supposedly fixed way back with Groovy 2.4.5 released in September 2015, so I'm not entirely sure it'll work, but maybe worth a try? (Though when searching that property I come across more recent things like Jenkins having a problem fixed by playing around with that flag back in 2019: https://issues.jenkins.io/browse/JENKINS-50223)

ultraq commented 3 years ago

Also, something I remembered: if it does look like Groovy is the culprit here, then there is a Java-only fork of the layout dialect at https://github.com/zhanhb/thymeleaf-layout-dialect that might be worth a try. The dev has made some contributions back to this repo from their work in the Java-only fork, and it looks like they are keeping their fork up to date (their latest release is 2.5.2 just a few days ago, and given 2.5.3 was only an upgrade of the Groovy package they might not make a 2.5.3 so you won't be missing out on any features/patches as of writing).

Airforce111 commented 3 years ago

Thanks for the info. Much appreciated!

Didn't know there was a java fork. We'll give the two suggestions a try and see how it goes.

Airforce111 commented 3 years ago

Future fyi...

Linking to groovy source, to reveal its internal note. https://github.com/apache/groovy/blob/bf160bbaaf812fac9ffa246206e1b16b37ef4ac4/src/main/java/org/codehaus/groovy/reflection/GroovyClassValueFactory.java#L32

Seems there are some legacy tickets calling for the eventual removal of the -Dgroovy.use.classvalue flag, which hasn't gained much traction since there are those calling for the keeping of the flag, but default to true. https://issues.apache.org/jira/browse/GROOVY-8199

Airforce111 commented 3 years ago

An update.

We tried out the -Dgroovy.use.classvalue=false flag and it appears to have worked. There's no longer a strong reference related to ClassValue.

We did not try out the java only fork.

Thanks~!

ultraq commented 3 years ago

Awesome! I'll keep this issue open for a little bit longer just to remind me to finish my reading of those Jira tickets.