Problem with tests under 2017.1 - Too many projects leaked

Indeed, I did have some project leaks :-). Thanks!

Same things happened to us on test projects, and it seems like JavaCodeInsightFixtureTestCase isn't doing a good job at disposing of projects.

We had to dispose of the projects ourselves, on tearDown().

https://github.com/GoogleCloudPlatform/google-cloud-intellij/pull/1296/files

João Martins, so far everyone reporting this has conceded to have some project leaks. Have you looked at the snapshots and ensured they don't have leaks? Can you share those snapshots?

I've looked at the heap dump and it showed a bunch of existing references to the ProjectImpl objects, none of them related to our code. I'm not sure how to upload the heap dump on this forum -- it only seems possible to upload images.

I'm not claiming the leak isn't there, just that if it's there, it might not be being caused by us. We're not creating any project references anywhere that I can see.

Sorry for the lack of context on the PR -- it just shows that on EndpointTestBase, we need to manually dispose of the projects, that should have been previously disposed by IJ/JavaCodeInsightFixtureTestCase.tearDown().

Running the tests in that branch that extend EndpointTestBase, prior to that commit (or on that commit, minus the EndpointTestBase changes), will fail, on a flaky basis.

In dumps, one should look at paths from GC roots, excluding ones via weak/soft references.

If you still believe there's a bug, you can upload the dump (zipped) to https://uploads.services.jetbrains.com/ and post its file name here.

I'm not sure how to tell whether the references are from GC or non GC roots by looking at the below image. Are you able to tell?

There's one entry where there's no memory address after the class name (i.e., ProjectImpl). I'm guessing that's our non-GC root, but I suspect this is being caused by IJ.

At least, by looking at RestSignatureInspectionTest and EndpointTestBase, I can't see anywhere we're adding that reference explicitly.

https://github.com/GoogleCloudPlatform/google-cloud-intellij/blob/master/google-cloud-tools-plugin/testSrc/com/google/cloud/tools/intellij/appengine/validation/RestSignatureInspectionTest.java

https://github.com/GoogleCloudPlatform/google-cloud-intellij/blob/master/google-cloud-tools-plugin/testSrc/com/google/cloud/tools/intellij/appengine/validation/EndpointTestBase.java

Yes, from this picture nothing is obvious :) I'd analyze the snapshot with YourKit, if you share it.

Thanks for the help. I just uploaded heapDump2.hprof (sorry it isn't zipped).

FYI, removing the call

PsiFile file = myFixture.addFileToProject("/temp", "someFile");

seems to have stopped the issue for us.

It isn't a definitive fix in that anyone who uses this method in unit tests is probably running into the same thing. Since this file was seemingly not being used in any of the tests (they passed before/after without any other changes), there might be some issues tearing down the environment/fixtures in your API.

Actually, after investigating a bit more, what I'm seeing with YourKit is that there appear to be lingering references to ProjectImpl objects from a mock object. That would be more likely to be caused by the second line that we removed:

when(mockPsiMethod.getContainingFile()).thenReturn(file);

I added mockPsiMethod = null to the beginning of my tearDown() method and it seems to have worked. I haven't gotten any project leaks in a while.

But this begs the question, why isn't the mock being disposed of at tearDown time? We're not using Mockito's runner because we're using IJ's, and I don't know if Mockito does anything special with mocks at teardown time.

So it's a leak in the test. Test classes are instantiated at the beginning of the test run and retained through all test time. So it's a normal practice to null the fields in tearDown to avoid leaks. Mockito doesn't do this automatically, the test should do it. There's UsefulTestCase#clear(Declared)Fields to help with that, but someone should still call it.