You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-dev@axis.apache.org by "Volanis, Alexander" <AV...@rsasecurity.com> on 2004/05/31 07:22:02 UTC

Patch: Memory leak in DeserializationContextImpl.parce() fixed

Hello Axis Developers,

We use Axis 1.1 in one of our products and as a result of a number of
performance tests we identified a memory leak in
org.apache.axis.encoding.DeserializationContextImpl.parse() method. The
way the code was attempting to remove the reference to "this" from the
instance of SAXParser is just plain wrong. Looking into Xerces code for
the SAXParcerImpl.setProperty() method null is not an acceptable value.
I added a NullLexicalHandler inner class that can be used to remove the
reference and allow memory to be released. Although it is not
significant under light load conditions the complexity of the messages
and the load can cause the pool of SAXParser instances maintained by
XMLUtils to grow significantly. The amount of memory that is held by
each instance of DeserializationContextImpl can be significant as the
message size grows. Examining the system with Jprobe helped us identify
the source of the problem and make this change which corrected the
problem for our application using Axis 1.1 Final.

The attached unified diff is against the latest CVS for Axis 1.2. The
exact same code would work in 1.1 as well, the diff is trivial to apply.

I do not have CVS accees but I used the ViewCVS facility to download
version 1.81 of this file and make the changes. Would anyone of the
commiters care to verify this fix and apply?

Regards,
Alex Volanis


Re: Patch: Memory leak in DeserializationContextImpl.parce() fixed

Posted by Davanum Srinivas <da...@gmail.com>.
+1

On Mon, 31 May 2004 23:15:34 +0900, Ias <ia...@tmax.co.kr> wrote:
> > Ias,
> >
> > can we please add a switch to switch it on? as it may work in
> > some environments?
> 
> In my opinion, any SAX-compliant parser such as Xerces and Crimson should
> work as the description said, so setting NullLexicalHandler all the time
> seems good in terms of both scalable performance and proper usage.
> 
> Regards,
> 
> Ias
> 
> 
> 
> >
> > thanks,
> > dims
> >
> > On Mon, 31 May 2004 16:20:54 +0900, Ias <ia...@tmax.co.kr> wrote:
> > >
> > > > Hello Axis Developers,
> > > >
> > > > We use Axis 1.1 in one of our products and as a result of
> > a number
> > > > of performance tests we identified a memory leak in
> > > > org.apache.axis.encoding.DeserializationContextImpl.parse()
> > > > method. The way the code was attempting to remove the
> > reference to
> > > > "this" from the instance of SAXParser is just plain
> > wrong. Looking
> > > > into Xerces code for the
> > > > SAXParcerImpl.setProperty() method null is not an
> > acceptable value.
> > >
> > > FYI, according to
> > >
> > http://www.saxproject.org/apidoc/org/xml/sax/package-summary.html#pack
> > > age_de
> > > scription
> > >
> > > lexical-handler
> > >
> > > Used to see some syntax events that are essential in some
> > applications:
> > > comments, CDATA delimiters, selected general entity inclusions, and
> > > the start and end of the DTD (and declaration of document element
> > > name). The Object must implement org.xml.sax.ext.LexicalHandler.
> > >
> > > Therefore, null value is unacceptable for the property in
> > the sense above.
> > >
> > > Thanks,
> > >
> > > Ias
> > >
> > >
> > >
> > > > I added a NullLexicalHandler inner class that can be used
> > to remove
> > > > the reference and allow memory to be released.
> > > > Although it is not significant under light load conditions the
> > > > complexity of the messages and the load can cause the pool of
> > > > SAXParser instances maintained by XMLUtils to grow significantly.
> > > > The amount of memory that is held by each instance of
> > > > DeserializationContextImpl can be significant as the message size
> > > > grows. Examining the system with Jprobe helped us identify the
> > > > source of the problem and make this change which corrected the
> > > > problem for our application using Axis 1.1 Final.
> > > >
> > > > The attached unified diff is against the latest CVS for Axis 1.2.
> > > > The exact same code would work in 1.1 as well, the diff
> > is trivial
> > > > to apply.
> > > >
> > > > I do not have CVS accees but I used the ViewCVS facility
> > to download
> > > > version 1.81 of this file and make the changes.
> > > > Would anyone of the commiters care to verify this fix and apply?
> > > >
> > > > Regards,
> > > > Alex Volanis
> > > >
> > > >
> > >
> > >
> >
> 
>

RE: Patch: Memory leak in DeserializationContextImpl.parce() fixed

Posted by Ias <ia...@tmax.co.kr>.
> Ias,
> 
> can we please add a switch to switch it on? as it may work in 
> some environments?

In my opinion, any SAX-compliant parser such as Xerces and Crimson should
work as the description said, so setting NullLexicalHandler all the time
seems good in terms of both scalable performance and proper usage.

Regards,

Ias

> 
> thanks,
> dims
> 
> On Mon, 31 May 2004 16:20:54 +0900, Ias <ia...@tmax.co.kr> wrote:
> > 
> > > Hello Axis Developers,
> > >
> > > We use Axis 1.1 in one of our products and as a result of 
> a number 
> > > of performance tests we identified a memory leak in
> > > org.apache.axis.encoding.DeserializationContextImpl.parse()
> > > method. The way the code was attempting to remove the 
> reference to 
> > > "this" from the instance of SAXParser is just plain 
> wrong. Looking 
> > > into Xerces code for the
> > > SAXParcerImpl.setProperty() method null is not an 
> acceptable value.
> > 
> > FYI, according to
> > 
> http://www.saxproject.org/apidoc/org/xml/sax/package-summary.html#pack
> > age_de
> > scription
> > 
> > lexical-handler
> > 
> > Used to see some syntax events that are essential in some 
> applications:
> > comments, CDATA delimiters, selected general entity inclusions, and 
> > the start and end of the DTD (and declaration of document element 
> > name). The Object must implement org.xml.sax.ext.LexicalHandler.
> > 
> > Therefore, null value is unacceptable for the property in 
> the sense above.
> > 
> > Thanks,
> > 
> > Ias
> > 
> > 
> > 
> > > I added a NullLexicalHandler inner class that can be used 
> to remove 
> > > the reference and allow memory to be released.
> > > Although it is not significant under light load conditions the 
> > > complexity of the messages and the load can cause the pool of 
> > > SAXParser instances maintained by XMLUtils to grow significantly. 
> > > The amount of memory that is held by each instance of 
> > > DeserializationContextImpl can be significant as the message size 
> > > grows. Examining the system with Jprobe helped us identify the 
> > > source of the problem and make this change which corrected the 
> > > problem for our application using Axis 1.1 Final.
> > >
> > > The attached unified diff is against the latest CVS for Axis 1.2. 
> > > The exact same code would work in 1.1 as well, the diff 
> is trivial 
> > > to apply.
> > >
> > > I do not have CVS accees but I used the ViewCVS facility 
> to download 
> > > version 1.81 of this file and make the changes.
> > > Would anyone of the commiters care to verify this fix and apply?
> > >
> > > Regards,
> > > Alex Volanis
> > >
> > >
> > 
> >
> 


Re: Patch: Memory leak in DeserializationContextImpl.parce() fixed

Posted by Davanum Srinivas <da...@gmail.com>.
Ias,

can we please add a switch to switch it on? as it may work in some environments?

thanks,
dims

On Mon, 31 May 2004 16:20:54 +0900, Ias <ia...@tmax.co.kr> wrote:
> 
> > Hello Axis Developers,
> >
> > We use Axis 1.1 in one of our products and as a result of a
> > number of performance tests we identified a memory leak in
> > org.apache.axis.encoding.DeserializationContextImpl.parse()
> > method. The way the code was attempting to remove the
> > reference to "this" from the instance of SAXParser is just
> > plain wrong. Looking into Xerces code for the
> > SAXParcerImpl.setProperty() method null is not an acceptable value.
> 
> FYI, according to
> http://www.saxproject.org/apidoc/org/xml/sax/package-summary.html#package_de
> scription
> 
> lexical-handler
> 
> Used to see some syntax events that are essential in some applications:
> comments, CDATA delimiters, selected general entity inclusions, and the
> start and end of the DTD (and declaration of document element name). The
> Object must implement org.xml.sax.ext.LexicalHandler.
> 
> Therefore, null value is unacceptable for the property in the sense above.
> 
> Thanks,
> 
> Ias
> 
> 
> 
> > I added a NullLexicalHandler inner class that can be used to
> > remove the reference and allow memory to be released.
> > Although it is not significant under light load conditions
> > the complexity of the messages and the load can cause the
> > pool of SAXParser instances maintained by XMLUtils to grow
> > significantly. The amount of memory that is held by each
> > instance of DeserializationContextImpl can be significant as
> > the message size grows. Examining the system with Jprobe
> > helped us identify the source of the problem and make this
> > change which corrected the problem for our application using
> > Axis 1.1 Final.
> >
> > The attached unified diff is against the latest CVS for Axis
> > 1.2. The exact same code would work in 1.1 as well, the diff
> > is trivial to apply.
> >
> > I do not have CVS accees but I used the ViewCVS facility to
> > download version 1.81 of this file and make the changes.
> > Would anyone of the commiters care to verify this fix and apply?
> >
> > Regards,
> > Alex Volanis
> >
> >
> 
>

RE: Patch: Memory leak in DeserializationContextImpl.parce() fixed

Posted by Ias <ia...@tmax.co.kr>.
> Hello Axis Developers,
> 
> We use Axis 1.1 in one of our products and as a result of a 
> number of performance tests we identified a memory leak in
> org.apache.axis.encoding.DeserializationContextImpl.parse() 
> method. The way the code was attempting to remove the 
> reference to "this" from the instance of SAXParser is just 
> plain wrong. Looking into Xerces code for the 
> SAXParcerImpl.setProperty() method null is not an acceptable value.

FYI, according to
http://www.saxproject.org/apidoc/org/xml/sax/package-summary.html#package_de
scription 

lexical-handler 

Used to see some syntax events that are essential in some applications:
comments, CDATA delimiters, selected general entity inclusions, and the
start and end of the DTD (and declaration of document element name). The
Object must implement org.xml.sax.ext.LexicalHandler.  

Therefore, null value is unacceptable for the property in the sense above.

Thanks,

Ias

> I added a NullLexicalHandler inner class that can be used to 
> remove the reference and allow memory to be released. 
> Although it is not significant under light load conditions 
> the complexity of the messages and the load can cause the 
> pool of SAXParser instances maintained by XMLUtils to grow 
> significantly. The amount of memory that is held by each 
> instance of DeserializationContextImpl can be significant as 
> the message size grows. Examining the system with Jprobe 
> helped us identify the source of the problem and make this 
> change which corrected the problem for our application using 
> Axis 1.1 Final.
> 
> The attached unified diff is against the latest CVS for Axis 
> 1.2. The exact same code would work in 1.1 as well, the diff 
> is trivial to apply.
> 
> I do not have CVS accees but I used the ViewCVS facility to 
> download version 1.81 of this file and make the changes. 
> Would anyone of the commiters care to verify this fix and apply?
> 
> Regards,
> Alex Volanis
> 
>