Again: Xerces not server proof ?

["Cornelius, Martin" <Ma...@softwareag.com>]

Some weeks ago i posted a message on this list, where i pointed out that
Xerces, as it currently is, cannot survive temporary out-of-memory
conditions without punching memory leaks, and thus looks not really fit to
be used in a 'cruel-real-world' server application. ( The Subject was :
"Xerces in an application where fail of new must be tolerated :  Impossible
?" ) 

So far, i have seen no followup on this, and thus would like to ask the
development team: Are you at all interested to address this issue, or do you
think i'm completely wrong ?

Apologize my tedious manner, and thanks for any enlightening,

Martin

P.S, for reference, this is the original post:

> -----Original Message-----
> 
> Hi there,
> 
> we are currently reasoning about whether it is possible to 
> use Xerces in a
> multithreaded server Application, whrere failure of memory 
> allocation must
> be tolerated. More exactly: failure of global operator new or 
> malloc() may
> always happen when the server is heavy loaded, but they will 
> never terminate
> the server. Of course, the particular client request in whose 
> context the
> failed allocation happened must be aborted in this case. However, this
> abortion will proceed in a controlled way and MUST NEVER lead 
> to memory
> leaks.
> 
> AFAICS, the current coding of Xerces does not fulfill this 
> requirement. E,g,
> look at this code fragments from class QName:
> 
> class XMLUTIL_EXPORT QName
> {
> // ....
> private :
>     XMLCh*              fPrefix;
>     XMLCh*              fLocalPart;
>     XMLCh*              fRawName;
> };
> 
> fPrefix, fLocalPart, and fRawname are initialized to zero, 
> and later on
> global operator new[] is used to allocate buffers. In the destructor,
> cleanup() is called:
> 
> void QName::cleanUp()
> {
>     delete [] fLocalPart;
>     delete [] fPrefix;
>     delete [] fRawName;
> }
> 
> So far, so good. But now look at another invocation of new in 
> this member
> function:
> 
> void QName::setLocalPart(const XMLCh* localPart)
> {
>     unsigned int newLen;
> 
>     newLen = XMLString::stringLen(localPart);
>     if (!fLocalPartBufSz || (newLen > fLocalPartBufSz))
>     {
>         delete [] fLocalPart;
>         fLocalPartBufSz = newLen + 8;
>         fLocalPart = new XMLCh[fLocalPartBufSz + 1];
>                           ^^^
>     }
>     XMLString::moveChars(fLocalPart, localPart, newLen + 1);
> }
> 
> If this new fails and raises an exception, we run into 
> trouble: If we call
> Qname's destructor or call Qname::cleanUp directly, 
> fLocalPart is deleted
> twice, if we do not, we get two memory leaks for fPrefix and fRawname.
> Unfortunately, code like this seems to be present many times 
> in Xerces 8-(.
> Worse, Qname is a basic building block, and all users of Qname seem to
> ignore totally that Qname might throw bad_alloc.
> 
> These are several solutions i can currently imagine, orderded 
> by my personal
> preference:
> 
> 1 - inspect the complete code for possible memory leaks 
> caused by bad_alloc
> execeptions, and try to fix this using try/catch statements. 
> AFAICS, this is
> a quite daunting and error-prone task, as basic classes like 
> Qname are used
> in many places and may throw bad_alloc in most operations. 
> 
> 2 - Encourage users of Xerces to override global new and 
> delete if they want
> applications to survive bad_malloc. This does not require to 
> modify Xerces,
> but it may be undesirable or even impossible in many 
> circumstances. IMHO, a
> library designed for common use should not require it's users 
> to do this.
> 
> 3 - Throughout Xerces, replace ALL occurences of new and 
> delete by calls to
> suitable allocator classes/functions, and never use global 
> new and delete
> outside of this allocators. The IDOM has already made steps in this
> direction by using IDDocumentImpl as some kind of arena 
> allocator ( that
> does not support deletion ). Of course, this is also a boring 
> task, but in
> contrast to solution 1 the effort can be easily estimated 
> (simply count the
> new and delete invocations) and a result that fulfills the 
> requirement is
> guaranteed. Currently i think, this is the way to go.
> 
> Please, send me your comments !  Martin


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

Re: Again: Xerces not server proof ?

["Jason E. Stewart" <ja...@openinformatics.com>]

"Cornelius, Martin" <Ma...@softwareag.com> writes:

> Some weeks ago i posted a message on this list, where i pointed out
> that Xerces, as it currently is, cannot survive temporary
> out-of-memory conditions without punching memory leaks, and thus
> looks not really fit to be used in a 'cruel-real-world' server
> application. ( The Subject was : "Xerces in an application where
> fail of new must be tolerated : Impossible ?" )

I believe that all potential memory leaks should be cleaned up from
Xerces. I believe that it is important for Xerces to be used in server
environments for handling XML, and it won't be if there are leaks.

As far as how to fix the issues you've identified, I'll have to leave
that up to Tinny and the Toronto crew.

jas.

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

Re: Again: Xerces not server proof ?

[Tinny Ng <tn...@ca.ibm.com>]

Martin,

I agree that we should fix any potential memory leak that remains after
exception scenario, and that Xerces-C++ should be exception-safe.  In fact we do
try to eliminate them.   Bugzilla bug like 3565 was an example of such kind of
leak fix.   But our current approach is to fix them one by one whenever it is
discovered and reported in bugzilla by any users.

Regarding your earlier example in QName:

> void QName::setLocalPart(const XMLCh* localPart)
> {
>     unsigned int newLen;
>     newLen = XMLString::stringLen(localPart);
>     if (!fLocalPartBufSz || (newLen > fLocalPartBufSz))
>     {
>         delete [] fLocalPart;
>         fLocalPartBufSz = newLen + 8;
>         fLocalPart = new XMLCh[fLocalPartBufSz + 1];
>                           ^^^
>     }
>     XMLString::moveChars(fLocalPart, localPart, newLen + 1);
> }
> If this new fails and raises an exception, we run into trouble: If we call
> Qname's destructor or call Qname::cleanUp directly, fLocalPart is deleted
> twice, if we do not, we get two memory leaks for fPrefix and fRawname.

I think the fix is to reset fLocalPart to zero before calling new, i.e.

    if (!fLocalPartBufSz || (newLen > fLocalPartBufSz))
    {
        delete [] fLocalPart;
        fLocalPartBufSz = newLen + 8;
/* =>add this ==>*/    fLocalPart = 0;
        fLocalPart = new XMLCh[fLocalPartBufSz + 1];
                          ^^^
    }

Then deleting fLocalPart twice is not a problem, and users can call
QName::cleanUp().

But generally speaking, I agree that there may be better way to redesign the new
/ delete mechanism in Xerces-C++.  But as a 'cruel-real-world' server
application, performance is also an important issue.   And putting try / catch
block everywhere when new is called (the Sol'n #1) will definitely hit
performance badly.   And the IDOM approach (Sol'n #3) is only good for
single-thread scenario.  To support multi-thread scenario, we have to adopt some
sort of locking mechanism which may have an impact on performance as well.

I think we need to gather more design suggestion from the Open Source Community
for the best proposal.   Any suggestion / feedback from the mailing list is
welcome.

But in the meantime, as a short term solution, please report any findings (like
the QName problem that you found) in bugzilla, and we can address each of them
individually.

Thanks!

Tinny





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