Patch JGit FS class to prevent classloader memory leak #2544
Conversation
When using the rewrite-gradle-plugin, JGit's FS class adds a Runtime shutdown hook that inadvertently creates a shadow reference to the RewriteClassLoader of the plugin's classpath isolation feature. This expands to effectively mean that each time the rewrite configuration is modified and a new RewriteClassLoader is created, the previous one is leaked on the heap without ever being able to be reclaimed by the garbage collector. The included patch here removes just the shutdown hook, so that we can stop leaking the classloader
|
This fixes openrewrite/rewrite-gradle-plugin gh-132. |
|
@shanman190 Was a corresponding issue reported upstream? It would be nice if this patch could be removed at some point. |
|
@knutwannheden, I hadn't yet as this was something that @sambsnyd and I found when running with the Gradle daemon. Based on the upstream implementation, I suspect that JGit was really never intended to be run within an isolated classloader like we're doing for rewrite (there are a lot of runtime shutdown hooks beyond this one). I can see if I can't figure out how to file an issue on eclipse's bugzilla to see what insight we can get from them. |
|
Sounds good. I happen to know one of the JGit committers. If you like I could ask him about his opinion on this use case. |
|
@knutwannheden, yeah, if you've got an in that would be great to see what we can glean. Thanks! |
|
@knutwannheden, did you ever hear back from that JGit committer? |
|
@shanman190 He mentioned that he was rather busy at the moment and wouldn't have time to look into it. But I filed a ticket and we could look into submitting a change (they use Gerrit and not GitHub). |
When using the rewrite-gradle-plugin, JGit's FS class adds a Runtime shutdown hook that inadvertently creates a shadow reference to the RewriteClassLoader of the plugin's classpath isolation feature. This expands to effectively mean that each time the rewrite configuration is modified and a new RewriteClassLoader is created, the previous one is leaked on the heap without ever being able to be reclaimed by the garbage collector. The included patch here removes just the shutdown hook, so that we can stop leaking the classloader