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

[Charl P. Botha]

On Thu, 2004-03-25 at 15:43, Brad King wrote:
> Charl P. Botha wrote:
> 
> > Dear developers,
> > 
> > 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.

Okay, so if I understand you correctly, the programmer using ITK should
realise that the ITK error handling doesn't trap memory allocation
errors (is this documented somewhere ITK-wise?) and then use
_set_new_handler() (or whatever's applicable) him/herself?  I understand
the philosophy about ITK being a library, but I very humbly disagree
with ITK not trapping deep-down memory allocation errors.  At the very
least, it could throw an itk-style exception to keep consistent with
other error-handling in the library.

What's wrong with checking the new[] for a return value on all systems? 
On systems that throw exceptions at allocation, it would bail before the
NULL check.  On systems that don't, the NULL check would do its job.  No
global settings required.

Maybe something like:
try {
  blaat = new PType[n];
  if (blaat == NULL)
    itkExceptionMacro(...);
}
except bad_alloc &e
{
  itkExceptionMacro(...);
}

This way, ITK has consistent behaviour on all systems independent of
new[] exception behaviour.  Oh wait, this is exactly what you propose!
:) except that you want to throw bad_alloc instead of using
itkExceptionMacro().  The advantage of itkExceptionMacro is that you
know where in the code it happened...

Thanks very much,
Charl

-- 
charl p. botha http://cpbotha.net/ http://visualisation.tudelft.nl/