FileSystem#currentDir threadlocal classloader leak

Question

I started to load plantuml.jar in its own classsloader, so that users can easily use any version they want (in IntelliJ https://github.com/esteinberg/plantuml4idea ). I could also dispose of it when the IDE has not enough memory and it is not being used.

The problem is that FileSystem#currentDir holds net.sourceforge.plantuml.security.SFile, which prevents the classloader unloading. I could workaround it by either clearing it via reflection, or use my own threads which I can kill. But I think it would be more proper to simply store java.io.File, or an absolute path as a String, not SFile.

smetana.core.Z#instances2  seems to be cleared, so that is not an issue.

Answer 1

Interesting :-)

I *think* that the real issue comes from the fact we are using a ThreadLocal.

I may be wrong, but even if we would use ThreadLocal<String> or ThreadLocal<File> you would have the same issue.

Removing this ThreadLocal will not be easy... In the mean time, we've just published a new beta http://beta.plantuml.net/plantuml.jar

With this beta, you can do call FileSystem.getInstance().resetThreadLocals()

Could you test and tell us if it helps ?

Thanks !

Comments

It does not work. 

It seems that: 

currentDir = new ThreadLocal();

- does not get rid of it, you have to call: 

currentDir.remove(); 

I made an example and also checked it with Eclipse Memory Analyzer. 

You can see how it affects the the size of heapdumps....

https://gist.github.com/krasa/52a8325dbe95d549fcb246ba8693494e

But not only that, it seems that it causes leaking where it would not be otherwise, you can test it in this IntelliJ project:

https://github.com/krasa/ThreadLocal-Classloader-Leak/blob/master/src/com/company/StrangeLeaks.java#L42

ThreadLocal<String> or ThreadLocal<File> would help, because it does not hold a reference to a class from plantuml jar. The content would still leak, but that would be a negligable. Also nobody would have to manually call clearing methods. I tested that with String and it works fine, an easy change, no need to get rid of ThreadLocals.

https://github.com/plantuml/plantuml/compare/master...krasa:leak?expand=1

---

ThreadLocal<String> or ThreadLocal<File> would help, because it does not hold a reference to a class from plantuml jar. 

Of course : I understand. Thanks for your explanation.

(EDIT : We now are using ThreadLocal<String> as in your example. Thanks !)

So here is a new beta http://beta.plantuml.net/plantuml.jar which now uses ThreadLocal<String>.

Tell us if it helps !

---

Works fine, Thanks!