1 Mar
2010
1 Mar
'10
11:50 a.m.
On Mar 1, 2010, at 11:42 AM, Caleb James DeLisle wrote:
Vincent Massol wrote:
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
On Mar 1, 2010, at 11:18 AM, Caleb James DeLisle
wrote:
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.
Vincent Massol wrote:
Then the question is why do we reference the context statically? Can't we remove
that? 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. >>>> Jira issue:
http://jira.xwiki.org/jira/browse/XWIKI-4953
>>>> Patch:
>>>>
http://jira.xwiki.org/jira/secure/attachment/16788/XWIKI-4496-patchTestMemo…
>>>>
>>>> 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();
>>>> }
>>>>