Fix memory leak when ThreadLocal are used inside tested code

[LudoPL]

This is a proposal of a fix (or a beginning of a fix) for https://github.com/google/gwtmockito/issues/53

Explanation of the issue (sorry if it's not very clear): Mockito is using the ThreadLocal java feature to keep a context with instances of MockitoConfiguration and MockingProgressImpl between calls. GwtMockitoTestRunner set a specific ClassLoader in the TreadContext before running each test and restore the previous one at the end. The JVM ThreadLocal implementation stored the thread contextual information in a map in the thread where the key is a ThreadLocal instance and the value the object stored. These ThreadLocal instances are referenced statically by two mockito classes loaded through the GwtMockitoClassLoader. This mean these instances can not be garbage collected until the GwtMockitoClassLoader that loaded them is. It also mean, as there is several instances of the GwtMockitoClassLoader, a same static field definition in the source code will reference a different object in the different version of the classes loaded in the different class loaders. These instances can not be garbage collected until the GwtMockitoClassLoader that loaded them is. So as there is several instances of the GwtMockitoClassLoader, a same static field definition in the source code will reference a different object in the different version of the classes loaded in the different class loaders. The entries in the ThreadLocalMap won't be removed from the Map while the key is not garbage collected (a weak reference is used in the Map for the key so it's not the map itself that directly prevent the garbage collection of the key). So the DefaultMockitoConfiguration and MockingProgressImpl objects stored as value in the ThreadLocalMap won't be removed until the GwtMockitoClassLoader that created their associated map keys is garbage collected (the value of the ThreadLocalMap are note weakly referenced themselves). The probem is that these map value objects are them selves loaded by the GwtMockitoClassLoader. So they prevent the garbage collection of the GwtMockitoClassLoader instances that created. Thus there is a references loop and memory leak thak keeps all the GwtMockitoClassLoader created for each tests in the memory stack of the thread that run them. In order to break this references loop, the patch proposes to remove all values loaded by the gwtMockitoClassLoader and stored in the ThreadLocalMap at the end of the test run.

Note about the patch:

• In order to do that, I access the private threadLocals attribute of the JVM Thread class, but this fields can be dependent of the JVM implementation (and version?) so in that case the an AssertionError will be raised. May not be the best behavior...
• For the Unit Test, I have done it to allow you to reproduce the issue easily and to better understand the fix. But this test is dependent of the JVM GC implementation so do not hesitate to remove if you prefer or it if you observed issue with it.

Modifications done in the patch:

• remove objects attached to the GwtMockitoClassLoader in the Thread ThreadLocalMap at the end of the test execution
• add a unit test to illustrate the memory leak issue.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[LudoPL]

I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[ekuefler]

This is some awesome investigation! Thanks so much for figuring out what was going on and putting together a fix. It's obviously a huge hack, but everything about GwtMockito is a huge hack, so that's not a dealbreaker :)

I think it makes sense to allow what you've defined here as an experimental opt-in feature. Like you said, tying all GwtMockito tests to the implementation details of Thread probably isn't the best idea, and it's probably only a very small percentage of users who are having problems with memory leaks. So here's what I'd suggest:

• Define a new annotation like @WithExperimentalGarbageCollection, and have this logic apply only if the test class has that annotation. You can document on that annotation what exactly it does and what the caveats are.
• Move your implementation code to a package-private class like ThreadLocalCleaner just to keep it separate from the existing business logic of the runner.
• Keep your unit test, but move it to a separate test class like GwtMockitoMemoryLeakTest so we can isolate it easily if we find it misbehaves in certain contexts/JVMs.

How does that sound? Thanks again for looking at it!

[LudoPL]

Thanks for the remarks, I have tried to implement them, hope it will match what you have in mind. You are right it is better to make this clean optional and to isolate the logic ! Do not hesitate to do or ask for other modifications. And also thanks for your project, it is very useful !

[ekuefler]

Thanks again for working on this!

[cinlloc]

Thanks for this PR!