QIICR / dcmqi

dcmqi (DICOM for Quantitative Imaging) is a free, open source C++ library for conversion between imaging research formats and the standard DICOM representation for image analysis results
https://qiicr.gitbook.io/dcmqi-guide/
BSD 3-Clause "New" or "Revised" License
238 stars 62 forks source link

Memory allocation error with large number of segments #289

Open fedorov opened 7 years ago

fedorov commented 7 years ago

As reported by a user, there is an example where dcmqi SEG converter crashes with a memory allocation failure. The dataset to reproduce the issue has been provided. The crash stack is below (crash appears to happen in DCMTK):

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_CRASH (SIGABRT)
Exception Codes:       0x0000000000000000, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Application Specific Information:
abort() called
terminating with uncaught exception of type std::bad_alloc: std::bad_alloc

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib              0x00007fff9dca7d42 __pthread_kill + 10
1   libsystem_pthread.dylib             0x00007fff9dd95457 pthread_kill + 90
2   libsystem_c.dylib                   0x00007fff9dc0d420 abort + 129
3   libc++abi.dylib                     0x00007fff9c76094a abort_message + 266
4   libc++abi.dylib                     0x00007fff9c785c17 default_terminate_handler() + 243
5   libobjc.A.dylib                     0x00007fff9d295713 _objc_terminate() + 124
6   libc++abi.dylib                     0x00007fff9c782d49 std::__terminate(void (*)()) + 8
7   libc++abi.dylib                     0x00007fff9c7827be __cxa_throw + 121
8   libc++abi.dylib                     0x00007fff9c782e47 operator new(unsigned long) + 87
9   itkimage2segimage                   0x000000010ffb0475 DcmSegmentation::writeBinaryFrames(DcmItem&) + 485
10  itkimage2segimage                   0x000000010ffb21de DcmSegmentation::writeSegmentationImageModule(DcmItem&) + 6878
11  itkimage2segimage                   0x000000010ffa5f79 DcmSegmentation::write(DcmItem&) + 1369
12  itkimage2segimage                   0x000000010ffafa46 DcmSegmentation::writeDataset(DcmItem&) + 54
13  itkimage2segimage                   0x000000010fa8dc54 dcmqi::ImageSEGConverter::itkimage2dcmSegmentation(std::__1::vector<DcmDataset*, std::__1::allocator<DcmDataset*> >, std::__1::vector<itk::SmartPointer<itk::Image<short, 3u> >, std::__1::allocator<itk::SmartPointer<itk::Image<short, 3u> > > >, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool) + 47860
14  itkimage2segimage                   0x000000010fa17a70 ModuleEntryPoint + 62768
15  itkimage2segimage                   0x000000010fa00d02 main + 34
16  libdyld.dylib                       0x00007fff9db79235 start + 1
fedorov commented 7 years ago

Further investigation showed that:

1) it was possible to complete the conversion task by using --skip flag to the converter, which does not encode empty frames. The conversion process was also a lot faster.

2) I was able to reproduce the reported memory allocation problem after a very very long wait in the end of the conversion process:

itkimage2segimage(28547,0x7fffb50373c0) malloc: *** mach_vm_map(size=2305843009052147712) failed (error code=3)
*** error: can't allocate region
*** set a breakpoint in malloc_error_break to debug
libc++abi.dylib: terminating with uncaught exception of type std::bad_alloc: std::bad_alloc

@michaelonken my plan is to investigate this by adding valgrind tests and first making sure there are no memory leaks in the dcmqi code. Once that is done, we can look into DCMTK if the problem still persists. I am sure there are memory leaks in dcmqi! Let me know if you have a better suggestion!

fedorov commented 5 years ago

from @michaelonken

I fixed some variable declarations/computations when the GE aviation team reported problems with their large segmentation objects, too.

The fixes have been applied in June 2018, and it seems to me dcmqi master is based on DCMTK 3.6.2 from July 2017, and the github issue is from August 2017.

Maybe it's worth to try updating DCMTK (3.6.3, 3.6.4?) to see whether it fixes the problem.

Indeed, need to test with the current DCMTK master.

fedorov commented 5 years ago

@michaelonken indeed - there is no crash with the current DCMTK master! I will need to update dcmqi (it is more than one line change, since I need to re-do pre-built DCMTK binary we use for Appveyor). Thank you for pointing this out!