[Insight-developers] serious problem with new[] on MSVC++

[Brad King]

Charl P. Botha wrote:
> Dear developers,
> 
> Brad King wrote:
> 
>> Charl P. Botha wrote:
> 
> 
>>> It seems that, at least on MSVC, new (or new[]) does not throw an
>>> exception when failing to allocate memory[1][2].  Looking at for
>>> instance itk::ImportImageContainer::Reserve(), which does all the
>>> allocation for itk::Image (and thus for MANY of the ITK filters and
>>> sources), the return value of the new[] is not checked!
>>>
>>> I've seen this crash my applications on windows when working with large
>>> DICOM datasets.  The program silently continues running, until a filter
>>> writes to for example the output Image which has a NULL buffer (due to a
>>> failed new[] that wasn't caught).
>>>
>>> This can be fixed by installing a global new_handler function[3].  I've
>>> grepped through the ITK sources and couldn't find an invocation of
>>> _set_new_handler(), so I'm assuming no-one has done this.
>>>
>>> Any discussion (or corrections!) on this is welcome.  I'm not sure how
>>> new[] indicates failure on for instance the GCC compilers, but whatever
>>> the case may be, it's quite critical that this is handled *correctly* in
>>> ITK.  The ISO C++ spec says that new[] should throw (as far as I can
>>> see), but compilers don't always follow this to the letter, as is shown
>>> by this example.
>>
>>
>>
>> The C++ standard says that if a new or new[] operator is declared with 
>> an empty "throw()" specification then it should return NULL when 
>> failing to allocate memory.  Otherwise it should throw a 
>> std::bad_alloc exception.
>>
>> ITK has been written to assume that the exception is thrown and 
>> therefore never checks the results of allocation (which is the whole 
>> point of a throwing allocation routine).  However, ITK should not set 
>> anything that is of global process scope since it is a library.  The 
>> application should set the handler correctly if it wants to be robust 
>> to this problem.
>>
>> Another alternative is to send all ITK memory allocation through some 
>> function that tries to use new or new[] to alloate memory and then 
>> throws std::bad_alloc if it gets a NULL pointer back.  That way we can 
>> be sure the exception will be thrown no matter the default behavior 
>> provided by a compiler.
> 
> 
> Excuse the long quote, but this has been quite a while.
> 
> I just got bitten by this one again.  It's quite nasty: one's 
> application simply aborts and after the prerequisite debugging, one 
> realises that it's because MSVC++ 6.0 sp6 doesn't throw an exception 
> when a new[] allocation fails.  I'm not sure if .NET 2003 does the Right 
> Thing(tm).
> 
> Brad's proposed solution would definitely do the trick: checking the new 
> or new[] return value and throwing std::bad_alloc in the case of a NULL 
> in for instance itk::ImportImageContainer (this is where all itk::Image 
> allocation seems to take place) would do no harm on any of the currently 
> working C++ compilers, but would make life much easier with compilers 
> which do not raise exceptions for failing new invocations.

I've modified ImportImageContainer to throw itk::MemoryAllocationError 
when memory allocation fails.  We may want to change it to 
std::bad_alloc, but this approach requires fewer catch blocks in ITK 
programs.  Most of the tests catch only itk::ExceptionObject (and 
subclasses).

-Brad