[omniORB] _CORBA_String_helper - inline a good idea and/or on purpose?

[Martin Trappel]

Duncan Grisby wrote:
> On Friday 21 August, Martin Trappel wrote:
> 
> [...]
>> The problem was that we were passing the result of CORBA::string_dup
>> to a CosNaming::Name structure and when this went out-of-scope the
>> string was freed using _CORBA_String_helper.
>> _CORBA_String_helper is defined inline and ~_CORBA_Unbounded_Sequence
>> from which CosNaming::Name is derived is also inline, but
>> CORBA::String_dup is not inline, it's inside the omniORB DLL.
>>
>> So what happended was that the memory that came from CORBA::string_dup
>> was actually allocated by the VC8 runtime, but when this memory was
>> freed by CosNaming::Name the VC9 runtime was used ==> Heap corruption.
>>
>> So the question really is (apart from whether its a good idea to use a
>> VC8 compile of omniORB with a VC9 app):
>> Would it make sense to make _CORBA_String_helper non-inline to prevent
>> this kind of error?
> 
> You're right that changing _CORBA_String_helper would fix that
> particular problem. However, there are other places where the C++
> mapping makes it fundamentally impossible to mix memory allocators.

OK. This is kind of what I expected to hear. So I'll just keep that in 
mind if the question comes up in a non-testing environment.

> Take the NamingContext list() operation, for example. That returns (in
> an out argument) a BindingList that is allocated by the ORB, and must be
> deleted by the application.
> 

Hmm ... just to dig a bit deeper: Everything that has a virtual dtor on 
Windows should actually be delete-able without problems, since using a 
virtual dtor will not only call the correct ~Dtor() but also use the 
library to free the memory the object was allocated with.

> _CORBA_String_helper is the way it is because it means that applications
> that try to mix heaps fall over very quickly, which is better than the
> more subtle effects you'd get otherwise.
>

I agree. Falling over ever earlier would even be better :-)

cheers,
Martin