Memory management bugs

2010-08-03 Thread Lyublena Antova
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

2010-08-03 Thread David Bertoni

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-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org



Re: Memory management bugs

2010-08-04 Thread Lyublena Antova



> > 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

2010-08-07 Thread David Bertoni

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-unsubscr...@xerces.apache.org
For additional commands, e-mail: c-dev-h...@xerces.apache.org