[jira] Commented: (XALANJ-2195) Memory leak in XMLReaderManager

["Sébastien Tardif (JIRA)" <xa...@xml.apache.org>]

    [ http://issues.apache.org/jira/browse/XALANJ-2195?page=comments#action_12451748 ] 
            
Sébastien Tardif commented on XALANJ-2195:
------------------------------------------

The fix provided is trying to do the same thing as removing all the caching logic that the class is doing. So the real fix is to remove all the caching logics. So that m_readers and m_inUse class variable should be removed. The side effect is that synchronization is not needed anymore.

The caching was an optimization that introduce too many problems:

- in the majority of cases, application that deal with XML will be lot slower "playing with the XML" than creating an instance of XMLReader, so the optimization is useless
- if an application want to cache something, they can cache other places or use AOP to do the same caching that was done
- they is no official API in XMLReader to reset the XMLReader so that it can be cached but without its previous state
- SAXParser has reset method but doesn't clean all the previous state in particular the one taking lot of memory
- synchronization is a limiting factor for scalability
- if a caller forget to put a finally block to release the XMLReader more important memory leak will occur, so the design was fragile

> Memory leak in XMLReaderManager
> -------------------------------
>
>                 Key: XALANJ-2195
>                 URL: http://issues.apache.org/jira/browse/XALANJ-2195
>             Project: XalanJ2
>          Issue Type: Bug
>          Components: Xalan
>    Affects Versions: 2.7
>            Reporter: Marko Strukelj
>         Attachments: gc-roots.jpg, retained-object-sizes.jpg
>
>
> In class org.apache.xml.utils.XMLReaderManager 
> getXMLReader() method creates a new XMLReader (i.e. SAXParser) and stores it into ThreadLocal.
> releaseXMLReader() does not remove (set to null) ThreadLocal thus creating a permanent leak.
> Unfortunately the size of the cached Reader is typically dependent upon the size of the XML document you process (depends on implementation but this is the case with xerces SAXParser). In heavy load server environments with thread pools of tens and hundreds of threads the server sustains a significant memory leak (hundreds of megabytes - depending on the XML document sizes and number of threads in a thread pools).
> A fix is trivial:
> Put the following line at the end of releaseXMLReader method:
> m_readers.set(null);
> I wonder, why is reader stored in ThreadLocal in the first place?

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

---------------------------------------------------------------------
To unsubscribe, e-mail: xalan-dev-unsubscribe@xml.apache.org
For additional commands, e-mail: xalan-dev-help@xml.apache.org