You are viewing a plain text version of this content. The canonical link for it is here.
Posted to c-dev@xerces.apache.org by Lyublena Antova <an...@greenplum.com> on 2010/08/04 00:44:00 UTC
Memory management bugs
I tried to use Xerces with the pluggable MemoryManager and I discovered that on several occasions objects are instantiated with the global new operator that does not use the memory manager. Here are some of those cases:
* initializing the EncodingValidator in EncodingValidator.cpp
* creating a DOMImplementationListImpl in DOMImplementationImpl.cpp and DOMImplementationRegistry.cpp
* creating a DOMNodeListImpl in DOMNodeImpl.cpp
* creating a DOMDocumentTypeImpl in DOMImplementationImpl.cpp
* ...
In our code we essentially forbid the use of plain global "new" so the above cases blow up when Xerces is linked against our codebase.
To my understanding the pluggable memory manager is used either:
* by making classes derive from the XMemory class which overloads new and delete, or
* by using the global overloaded placement new operators that take a DomDocument(Impl) object
The problem classes mentioned above are not derived from XMemory but occasionally get instantiated with a plain "new" operator instead of the placement "new"-s.
I have a fix that makes those classes inherit the XMemory class, and thus get instantiated with the global memory manager. That caused some problems because on some occasions the global placement "new"-s were shadowed by the Xmemory member "new"-s which produced unexpected results. The solution was to force the use of the global new (::new) to avoid wrong resolving of operator calls.
Was there any reason why the classes above do not inherit from XMemory in the first place?
On a broader note, is there a particular reason why not have a placement new operator that takes a MemoryManager instance? Perhaps deallocation issues?
Thanks,
Lyublena
Re: Memory management bugs
Posted by David Bertoni <db...@apache.org>.
On 8/4/2010 10:42 AM, Lyublena Antova wrote:
>
>
>
> > > I have a fix that makes those classes inherit the XMemory class, and
> > > thus get instantiated with the global memory manager. That
> caused some
> > > problems because on some occasions the global placement “new”-s were
> > > shadowed by the Xmemory member “new”-s which produced unexpected
> > > results. The solution was to force the use of the global new
> (::new) to
> > > avoid wrong resolving of operator calls.
>
> > Just out of curiousity, can you provide an example of where this
> > occurred in the Xerces-C code?
>
> One of the cases was the following initialization in
> DomDocumentImpl.cpp, the DOMDeepNodeListImpl object was initialized
> with the wrong new operator:
>
> XMLSize_t id = fNodeListPool->put((*void**) rootNode, (XMLCh*) tagName,
> 0, *new* (*this*) DOMDeepNodeListImpl(rootNode, tagName));
>
>
> > > Was there any reason why the classes above do not inherit from
> XMemory
> > > in the first place?
> > Just inheriting from XMemory isn't always the right fix. In some
> cases,
> > there may be an available MemoryManager instance, either as a function
> > parameter or as a class member. It may also have been done that way to
> > avoid multiple inheritance, particularly in the DOM implementation
> classes.
>
> In the cases I encountered there didn’t seem to be an instance of a
> memory manager, except for XMLPlatformUtils::fgMemoryManager but there
> isn’t a global new operator that takes this one as an argument, thus the
> code for Xmemory::new would have been repeated.
> As for the multiple inheritance, it’s used all over the place so I doubt
> it’s that :)
I should have been more explicit about what I meant. If XMemory is not a
public-visible base class in the hierarchy, it defeats the purpose of
using it.
Imagine you have an abstract base class where XMemory is not visible,
then you introduce it in an implementation class. Should an object ever
be deleted through a pointer to the base class, the compiler won't know
to call XMemory::operator delete(), which will result in undefined behavior.
For this to work properly with the DOM classes, the DOM abstract bases
classes would need to be derived from XMemory.
At any rate, the best way to move forward to getting this fixed is to
create a Jira issue and attach a patch if possible.
Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org
Re: Memory management bugs
Posted by Lyublena Antova <an...@greenplum.com>.
> > I have a fix that makes those classes inherit the XMemory class, and
> > thus get instantiated with the global memory manager. That caused some
> > problems because on some occasions the global placement "new"-s were
> > shadowed by the Xmemory member "new"-s which produced unexpected
> > results. The solution was to force the use of the global new (::new) to
> > avoid wrong resolving of operator calls.
> Just out of curiousity, can you provide an example of where this
> occurred in the Xerces-C code?
One of the cases was the following initialization in DomDocumentImpl.cpp, the DOMDeepNodeListImpl object was initialized with the wrong new operator:
XMLSize_t id = fNodeListPool->put((void*) rootNode, (XMLCh*) tagName, 0, new (this) DOMDeepNodeListImpl(rootNode, tagName));
> > Was there any reason why the classes above do not inherit from XMemory
> > in the first place?
> Just inheriting from XMemory isn't always the right fix. In some cases,
> there may be an available MemoryManager instance, either as a function
> parameter or as a class member. It may also have been done that way to
> avoid multiple inheritance, particularly in the DOM implementation classes.
In the cases I encountered there didn't seem to be an instance of a memory manager, except for XMLPlatformUtils::fgMemoryManager but there isn't a global new operator that takes this one as an argument, thus the code for Xmemory::new would have been repeated.
As for the multiple inheritance, it's used all over the place so I doubt it's that :)
Re: Memory management bugs
Posted by David Bertoni <db...@apache.org>.
On 8/3/2010 3:44 PM, Lyublena Antova wrote:
> I tried to use Xerces with the pluggable MemoryManager and I discovered
> that on several occasions objects are instantiated with the global new
> operator that does not use the memory manager. Here are some of those cases:
>
> * initializing the EncodingValidator in EncodingValidator.cpp
> * creating a DOMImplementationListImpl in DOMImplementationImpl.cpp
> and DOMImplementationRegistry.cpp
> * creating a DOMNodeListImpl in DOMNodeImpl.cpp
> * creating a DOMDocumentTypeImpl in DOMImplementationImpl.cpp
> * ...
>
>
> In our code we essentially forbid the use of plain global “new” so the
> above cases blow up when Xerces is linked against our codebase.
>
> To my understanding the pluggable memory manager is used either:
>
> * by making classes derive from the XMemory class which overloads
> new and delete, or
> * by using the global overloaded placement new operators that take a
> DomDocument(Impl) object
>
>
> The problem classes mentioned above are not derived from XMemory but
> occasionally get instantiated with a plain “new” operator instead of the
> placement “new”-s.
>
> I have a fix that makes those classes inherit the XMemory class, and
> thus get instantiated with the global memory manager. That caused some
> problems because on some occasions the global placement “new”-s were
> shadowed by the Xmemory member “new”-s which produced unexpected
> results. The solution was to force the use of the global new (::new) to
> avoid wrong resolving of operator calls.
Just out of curiousity, can you provide an example of where this
occurred in the Xerces-C code?
> Was there any reason why the classes above do not inherit from XMemory
> in the first place?
Just inheriting from XMemory isn't always the right fix. In some cases,
there may be an available MemoryManager instance, either as a function
parameter or as a class member. It may also have been done that way to
avoid multiple inheritance, particularly in the DOM implementation classes.
> On a broader note, is there a particular reason why not have a placement
> new operator that takes a MemoryManager instance? Perhaps deallocation
> issues?
Do you mean a global placement new operator? If so, I suspect the
deallocation issue is why it doesn't exist.
Dave
---------------------------------------------------------------------
To unsubscribe, e-mail: c-dev-unsubscribe@xerces.apache.org
For additional commands, e-mail: c-dev-help@xerces.apache.org