Issue Details (XML | Word | Printable)

Key: NUCCORE-552
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Yang ZHONG
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
DataNucleus Core

FetchGroup#planListeners leaks memory quickly soon Out Of Memory

Created: 23/Jul/10 06:53 PM   Updated: 04/Oct/12 05:02 PM   Resolved: 06/Oct/10 11:55 AM
Component/s: Persistence
Affects Version/s: 2.0.4
Fix Version/s: 2.2.0.m2

File Attachments: 1. Zip Archive NUCCORE-552.zip (4 kB)

Environment: Java 5, Linux

Severity: Production


 Description  « Hide
org.datanucleus.FetchGroup#planListeners holds strong references of org.datanucleus.FetchPlan, unfortunately, FetchGroup#deregisterListener is normally never invoked in RunTime. Since FetchPlan holds strong reference of org.datanucleus.ObjectManagerImpl which holds strong reference of org.datanucleus.jdo.JDOPersistenceManager, FetchGroup#planListeners leaks memory quickly soon Out Of Memory.

The Test Case to be attached, shows both FetchPlan & PersistenceManager still referenced after GC.

Just for the only purpose of Proof of Concept, following refinements seem solved the problem:
3-1. Add this into JDOPersistenceManager#close()
        fetchPlan.clearGroups();
3-2. Add this into org.datanucleus.jdo.JDOQuery#closeAll()
        if (fetchPlan != null)
            fetchPlan.clearGroups();
3-3. Add this into org.datanucleus.store.rdbms.query.legacy.JDOQLQuery
    public void closeAll()
    {
        super.closeAll();
        if (candidates instanceof CollectionCandidates)
        {
            ((CollectionCandidates)candidates).getFetchPlan().clearGroups();
        }
        else if (candidates instanceof Extent)
        {
            ((Extent)candidates).getFetchPlan().clearGroups();
        }
    }

Now the Test Case shows both FetchPlan & PersistenceManager GCed!

However, neither is user obligated to close()/clossAll(), nor should a JDO implementation count on so. Therefore, I personally think the real fix should be weakly referencing FetchGroup#planListeners elements, such as WeakHashMap#keySet().

After the real fix, both "pm.close();" & "query.closeAll();" should be commented out of the Test Case to verify the fix.

Given the weakly referencing fix, I personally still recommend above code change as they help GC.

Yang ZHONG made changes - 23/Jul/10 06:54 PM
Field Original Value New Value
Attachment NUCCORE-552.zip [ 11230 ]
Yang ZHONG added a comment - 23/Jul/10 07:04 PM
It seems the latest FetchGroup#planListeners (2.1.1) has the same problem of strongly referencing FetchPlan.

Yang ZHONG made changes - 23/Jul/10 07:14 PM
Attachment NUCCORE-552.zip [ 11230 ]
Yang ZHONG made changes - 23/Jul/10 07:14 PM
Attachment NUCCORE-552.zip [ 11231 ]
Yang ZHONG added a comment - 23/Jul/10 08:05 PM
Do we have other "listeners"? We may want to apply weakly referencing to all of them in general.

Andy Jefferson added a comment - 20/Sep/10 01:38 PM
As mentioned many times before, we have no time for "Proof of Concept". If you have some proposed fix then "patch" is the accepted format, nothing more.

Andy Jefferson made changes - 20/Sep/10 01:38 PM
Priority Blocker [ 1 ] Major [ 3 ]
Andy Jefferson added a comment - 06/Oct/10 11:55 AM
Added FetchPlan.clearGroups to Query.closeAll, and also to DefaultCandidateExtent.closeAll.
ObjectManager.close() already did clear out the fetch plan.
Can't reproduce any such problem.

Andy Jefferson made changes - 06/Oct/10 11:55 AM
Status Open [ 1 ] Resolved [ 5 ]
Resolution Fixed [ 1 ]
Andy Jefferson made changes - 06/Oct/10 11:55 AM
Fix Version/s 2.2.0.m2 [ 11022 ]
Andy Jefferson made changes - 20/Oct/10 09:29 AM
Status Resolved [ 5 ] Closed [ 6 ]
Andy Jefferson made changes - 04/Oct/12 05:02 PM
Component/s Persistence [ 10200 ]
Component/s JDO [ 10201 ]