Bug 550529 - org.eclipse.jgit.nls.NLS creates still a memory leak
Summary: org.eclipse.jgit.nls.NLS creates still a memory leak
Status: RESOLVED FIXED
Alias: None
Product: JGit
Classification: Technology
Component: JGit (show other bugs)
Version: 5.4   Edit
Hardware: PC Unix All
: P3 normal (vote)
Target Milestone: 5.8   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-28 07:06 EDT by Nils Ehmke CLA
Modified: 2020-06-05 05:30 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nils Ehmke CLA 2019-08-28 07:06:05 EDT
Hi, 

According to issue 449321 JGit should no longer contain the memory leak with the thread local. However, using either 4.11.0.201803080745-r or 5.4.2.201908231537-r I still get the same message from Tomcat9:

The web application [...] created a ThreadLocal with key of type [java.lang.InheritableThreadLocal] (value [java.lang.InheritableThreadLocal@4f02050e]) and a value of type [org.eclipse.jgit.nls.NLS] (value [org.eclipse.jgit.nls.NLS@3d06b420]) but failed to remove it when the web application was stopped. Threads are going to be renewed over time to try and avoid a probable memory leak.
Comment 1 Nils Ehmke CLA 2019-08-31 02:29:31 EDT
Hi,

I took another look at this. The main issue here is that a lot of the (if not all) interactions with JGit lead to the thread local being created. As JGit has (as far as I know) no knowledge of a potential servlet context it is running in, it cannot clean this up. I think I can avoid the memory leak warning from Tomcat by using the reflection API to get the thread local and call "remove" (either after each interaction in a finally block or with a request listener). However, this is of course only a workaround. Maybe it would be possible to add a public method to NLS to remove the thread local?
Comment 2 Nils Ehmke CLA 2019-09-02 04:36:28 EDT
Hi,

Another update (sorry for the multiple comments): It seems that my approach is not enough. There is at least one (worker) thread ("JGit-Upload-Pack") created by JGit, which I cannot access and which still creates the thread local. It would therefore also be necessary to add the remove-behaviour to such threads as well.
Comment 3 Eclipse Genie CLA 2019-12-19 20:24:21 EST
New Gerrit change created: https://git.eclipse.org/r/154858
Comment 4 Eclipse Genie CLA 2020-06-05 02:21:23 EDT
Gerrit change https://git.eclipse.org/r/154858 was merged to [master].
Commit: http://git.eclipse.org/c/jgit/jgit.git/commit/?id=aea95b819aeac1cce43e8ad78cef413e3cebaa08
Comment 5 Matthias Sohn CLA 2020-06-05 05:30:14 EDT
NLS now provides a clear() method to remove the ThreadLocal and clear the GlobalBundleCache