This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 206107 - Memory leak in StatisticsPanel / JUnitTestSession
Summary: Memory leak in StatisticsPanel / JUnitTestSession
Status: RESOLVED FIXED
Alias: None
Product: utilities
Classification: Unclassified
Component: Test Runner (show other bugs)
Version: 7.2
Hardware: PC Linux
: P2 normal (vote)
Assignee: Theofanis Oikonomou
URL:
Keywords: PERFORMANCE
Depends on:
Blocks:
 
Reported: 2011-12-07 19:55 UTC by Jesse Glick
Modified: 2012-06-12 13:14 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
proposed fix (5.67 KB, patch)
2012-06-06 15:48 UTC, Theofanis Oikonomou
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2011-12-07 19:55:26 UTC
After closing all open projects, I found 117 projects remaining in heap. One of them had this GC root back reference:

this	org.netbeans.modules.apisupport.project.NbModuleProject
project	org.netbeans.modules.junit.output.JUnitTestSession
session	org.netbeans.modules.gsf.testrunner.api.ResultDisplayHandler
displayHandler	org.netbeans.modules.gsf.testrunner.api.StatisticsPanel
leftComponent	javax.swing.JSplitPane
parent	javax.swing.JPanel
parent	org.netbeans.core.output2.OutputTab
value	java.util.HashMap$Entry
[8]	java.util.HashMap$Entry[]
table	java.util.HashMap
ioToTab	org.netbeans.core.output2.Controller
controller	org.netbeans.core.output2.Controller

Seems that some Test Results tab from an old test run was still holding a strong reference to the project being tested. It should not hold any such reference, at least not after the session has been finished. (Any references to source locations such as for Go to Source on stack trace lines should be retained just as URLs.)
Comment 1 Jesse Glick 2012-06-05 21:10:19 UTC
20120604-0da85416d886. Noticed high memory usage in the IDE despite all projects being closed. Taking a heap dump and searching for ProjectManager.ProjectStateImpl, I found eight projects still live in the heap. Three of them were held from ResultWindow:

this	org.netbeans.modules.java.j2seproject.J2SEProject	#1
project	org.netbeans.modules.junit.output.JUnitTestSession	#7
session	org.netbeans.modules.gsf.testrunner.api.ResultDisplayHandler	#12
displayHandler	org.netbeans.modules.gsf.testrunner.api.StatisticsPanel	#11
leftComponent	javax.swing.JSplitPane	#12
[4]	java.lang.Object[]	#54551 (10 items)
elementData	java.util.ArrayList	#26284
component	org.openide.awt.CloseButtonTabbedPane	#2
tabPane	org.netbeans.modules.gsf.testrunner.api.ResultWindow	#1
[1]	org.openide.windows.TopComponent[]	#18 (5 items)
...
Comment 2 Theofanis Oikonomou 2012-06-06 15:48:33 UTC
Created attachment 120438 [details]
proposed fix

Jesse, I am attaching a proposed fix. I have converted all strong references to Project that I could identify into WeakReference. Is this enough or did you mean something else? Thank you
Comment 3 Jesse Glick 2012-06-06 16:21:03 UTC
Looks like it should work. See if you can reproduce without the fix and thus confirm that it works. Probably suffices to open some projects, run some tests from them, close them, click memory button a few times to run GC, jps -lm | fgrep netbeans, jmap -dump:live,file=..., Profile > Load Heap Dump, filter on ProjectStateImpl, review any leaked instances to make sure they are not caused by the test runner.
Comment 4 Theofanis Oikonomou 2012-06-06 17:21:53 UTC
Jesse, thank you for the tip. I followed it and without the fix there were references to Project from test runner but with the fix I could not spot any. Fixed: http://hg.netbeans.org/core-main/rev/993c25f040ae
Comment 5 Jesse Glick 2012-06-06 18:53:24 UTC
Backed out in core-main #79de9f5db9b4 since this was an incompatible API change: TestMethodNode.project was protected. TBD exactly how to fix. Probably should switch it to protected Project getProject(), retaining just the URI as in other classes; need to then increment major release version of module, and update the dependency in all friend modules, fixing at least cnd.testrunner to use the new method instead of the field.
Comment 6 Theofanis Oikonomou 2012-06-07 12:33:00 UTC
Oups! Sorry for that. It turns out the affected modules that use the field are cnd.testrunner and maven.junit . Followed your suggestion: http://hg.netbeans.org/core-main/rev/cd42b07133ea
Comment 7 Jesse Glick 2012-06-07 15:58:55 UTC
Not enough to increment the version of gsf.testrunner and increment deps from those modules. Someone with a pre-cd42b07133ea installation accepting all updates in Plugin Manager will get the new version of gsf.testrunner while keeping the old versions of the two clients, leading to a NoSuchFieldError at runtime.

At a minimum you need to increment the spec versions of both cnd.testrunner and maven.junit to ensure that new versions calling getProject() are pushed to AU.

To be on the safe side (esp. w.r.t. non-netbeans.org modules like the Android test runner which you might not have checked), increment the major release version of gsf.testrunner to document the incompatibility of the change:

  OpenIDE-Module: org.netbeans.modules.gsf.testrunner/1

which means that all clients, even if otherwise unaffected by the change, must change their dependency to request relver 1 (and so also need to have new spec versions).
Comment 8 Theofanis Oikonomou 2012-06-07 16:33:39 UTC
(In reply to comment #7)
> Not enough to increment the version of gsf.testrunner and increment deps from
> those modules. Someone with a pre-cd42b07133ea installation accepting all
> updates in Plugin Manager will get the new version of gsf.testrunner while
> keeping the old versions of the two clients, leading to a NoSuchFieldError at
> runtime.
> 
> At a minimum you need to increment the spec versions of both cnd.testrunner and
> maven.junit to ensure that new versions calling getProject() are pushed to AU.

was not sure about that. Thank you for clarifying.

> 
> To be on the safe side (esp. w.r.t. non-netbeans.org modules like the Android
> test runner which you might not have checked), increment the major release
> version of gsf.testrunner to document the incompatibility of the change:
> 
>   OpenIDE-Module: org.netbeans.modules.gsf.testrunner/1
> 
> which means that all clients, even if otherwise unaffected by the change, must
> change their dependency to request relver 1 (and so also need to have new spec
> versions).

If I increase the Release version of gsf.testrunner do I still need to increment cnd.testrunner and maven.junit spec versions? And what about spec versions of the other friend modules? Does the spec versions for all of them need to be increased?
Comment 9 Jesse Glick 2012-06-07 17:11:55 UTC
(In reply to comment #8)
> If I increase the Release version of gsf.testrunner do I still need to
> increment cnd.testrunner and maven.junit spec versions?

Yes.

> And what about spec
> versions of the other friend modules? Does the spec versions for all of them
> need to be increased?

In this case, yes.

In a nutshell, if you have examined all of the declared friends - including non-netbeans.org modules such as ruby.testrunner etc. - and concluded that only cnd.testrunner and maven.junit are affected by the change, then you could take the shortcut of just updating the code in these two (as you did), making them depend on the new gsf.testrunner (as you did), and updating them too (as you still need to do). We are assuming here that anyone accepting the gsf.testrunner update would be accepting all updates, not picking and choosing modules to update, so they would get the new cnd.testrunner and/or maven.junit if they have these installed. Then other friends can be left alone.

But if you are unsure whether e.g. com.sun.tools.tuxedo.testrunner might be affected by the change, the only safe route is to assume that it is, and so to prevent LinkageError's you need to mark gsf.testrunner with a new major release version. When you do that, the module system will refuse to even load client modules depending on the previous major version. Therefore _all_ friends of gsf.testrunner in netbeans.org sources (main + contrib) need to be updated to request the new major release version, e.g. "1". (For the ones whose source code did not need to be modified, you can actually declare a range "0-1" which lets them link against either the old or new API.) And all these modules need to be "pushed" with a new spec version as well, so that existing installations get updated. Non-netbeans.org modules would get fixed up by their maintainers when next published.

[1] gives some background information. [2] has some newer policy, though more focused on stable public APIs (gsf.testrunner is still restricted). [3] proposes a simplified system that would make this sort of change less onerous.

[1] http://wiki.netbeans.org/VersioningPolicy#Incompatible_change
[2] http://wiki.netbeans.org/CompatibilityPolicy#Versioning_impact
[3] http://wiki.netbeans.org/NbmPackageStability#Handling_possibly_broken_dependencies
Comment 10 Quality Engineering 2012-06-08 06:09:45 UTC
Integrated into 'main-golden', will be available in build *201206080001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/993c25f040ae
User: Theofanis Oikonomou <theofanis@netbeans.org>
Log: Issue #206107 - Memory leak in StatisticsPanel / JUnitTestSession
Comment 11 Theofanis Oikonomou 2012-06-08 21:03:20 UTC
Followed the safe route:
http://hg.netbeans.org/main/rev/ba906667815a
http://hg.netbeans.org/main/contrib/rev/7d19ae4dd34e
Comment 12 Quality Engineering 2012-06-09 04:33:46 UTC
Integrated into 'main-golden', will be available in build *201206090001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/ba906667815a
User: Theofanis Oikonomou <theofanis@netbeans.org>
Log: Issue #206107 - Memory leak in StatisticsPanel / JUnitTestSession
Comment 13 Quality Engineering 2012-06-12 13:14:25 UTC
Integrated into 'main-golden', will be available in build *201206120719* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/4433bc40af62
User: Jesse Glick <jglick@netbeans.org>
Log: theofanis's #206107 continued.
Since ba906667815a was done well after cd42b07133ea, clients may have a 1.26 /0 gsf.testrunner
and then be presented with updates of clients (e.g. hudson 1.20) which cannot work with the installed gsf.testrunner,
yet there is no apparent gsf.testrunner update. Therefore need to push gsf.testrunner again.