On Mar 1, 2010, at 11:42 AM, Caleb James DeLisle wrote: > > > Vincent Massol wrote: >> On Mar 1, 2010, at 11:18 AM, Caleb James DeLisle wrote: >> >>> >>> Vincent Massol wrote: >>>> On Mar 1, 2010, at 10:48 AM, Caleb James DeLisle wrote: >>>> >>>>> By clearing the XWikiContext in the tearDown of the unit test, I can >>>>> stop the memory leak and stop running the tests in isolation. I am >>>>> proposing we apply this change because the XWikiContext is created >>>>> per test and the worst problem which can be created is tests >>>>> erroneously passing which I don't think is very likely from a change >>>>> like this. >>>> +1 for the main reason that this improves build times. The second reason >>>> is that it makes memory leaks more visible which gives us a better chance >>>> to find them (although this won't replace performance tests running over a >>>> few days for ex). >>>> >>>> However please add a comment just above the getContext().clear() to >>>> explain why this is needed. BTW why is it needed? :) >>> I will do that. The reason it's needed is because (it seems) the >>> context is referenced by a static variable which isn't cleared and >>> rather than pick through the tests looking for where the reference >>> was (and hoping no test ever does it again) I opted to clear the >>> context and let the tests use the context as they wish. This is my >>> understanding of the situation. >> >> Then the question is why do we reference the context statically? Can't we >> remove that? > > Static referencing is more a theory than a fact, the problem as I > see it is the context is used everywhere and referenced in a lot of > places so anything which is able to escape the tearDown process and > happens to hold a reference to the context will cause references to > persist pointing to everything. I think the choice is to either > clear the context or pour through each test individually looking for > any reference which might escape tearDown. I remember running > various tests individually and in pairs with very little memory and > finding a lot of them seem to have leaks.
Well the GC doesn't run all the time so you need to ensure you're forcing it to run after a test to know if the test has a leak. And yes, I couldn't get to the bottom of it in the past (didn't spend enough time) which is why I took the easy route of forking tests to isolate them. Thanks -Vincent >>>>> Jira issue: http://jira.xwiki.org/jira/browse/XWIKI-4953 >>>>> Patch: >>>>> http://jira.xwiki.org/jira/secure/attachment/16788/XWIKI-4496-patchTestMemoryLeak.patch >>>>> >>>>> Patch content: >>>>> >>>>> Index: core/xwiki-core/pom.xml >>>>> =================================================================== >>>>> --- core/xwiki-core/pom.xml (revision 27357) >>>>> +++ core/xwiki-core/pom.xml (working copy) >>>>> @@ -823,10 +823,6 @@ >>>>> <plugin> >>>>> <groupId>org.apache.maven.plugins</groupId> >>>>> <artifactId>maven-surefire-plugin</artifactId> >>>>> - <configuration> >>>>> - <!-- Prevent Out Of Memory errors resulting from tests >>>>> that do no free up the memory correctly --> >>>>> - <forkMode>pertest</forkMode> >>>>> - </configuration> >>>>> </plugin> >>>>> <!-- Publish a Core Test JAR --> >>>>> <plugin> >>>>> Index: >>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>> =================================================================== >>>>> --- >>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>> (revision 27357) >>>>> +++ >>>>> core/xwiki-core/src/test/java/com/xpn/xwiki/test/AbstractBridgedXWikiComponentTestCase.java >>>>> (working copy) >>>>> @@ -80,6 +80,7 @@ >>>>> protected void tearDown() throws Exception >>>>> { >>>>> Utils.setComponentManager(null); >>>>> + getContext().clear(); >>>>> super.tearDown(); >>>>> } >>>>> _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
