SlicerRt / SlicerRT

Open-source toolkit for radiation therapy research, an extension of 3D Slicer. Features include DICOM-RT import/export, dose volume histogram, dose accumulation, external beam planning (TPS), structure comparison and morphology, isodose line/surface generation, etc.
https://slicerrt.org
127 stars 60 forks source link

Crash when loading RT study #135

Closed cpinter closed 4 years ago

cpinter commented 4 years ago

When trying to load an RT study, then the application crashes at the examine stage. When the dose volume is examined, the corresponding plan's name is retrieved. The crash originates from the DICOM database during a fileForInstance call, and it goes dep into Qt. Here is the call stack:

    Qt5Cored.dll!qt_message_fatal(QtMsgType __formal, const QMessageLogContext & context, const QString & message) Line 1710    C++
    Qt5Cored.dll!QMessageLogger::fatal(const char * msg, ...) Line 822  C++
    Qt5Cored.dll!qt_assert(const char * assertion, const char * file, int line) Line 3126   C++
    qsqlited.dll!QList<int>::first() Line 345   C++
    qsqlited.dll!QSQLiteResult::exec() Line 491 C++
    Qt5Sqld.dll!QSqlQuery::exec() Line 1002 C++
    CTKDICOMCore.dll!ctkDICOMDatabase::fileForInstance(const QString sopInstanceUID) Line 2159  C++
>   vtkSlicerDicomRtImportExportModuleLogic.dll!vtkSlicerDicomRtImportExportModuleLogic::vtkInternal::ExamineRtDoseDataset(DcmDataset * dataset, OFString & name, std::vector<OFString,std::allocator<OFString>> & referencedSOPInstanceUIDs) Line 267  C++
    vtkSlicerDicomRtImportExportModuleLogic.dll!vtkSlicerDicomRtImportExportModuleLogic::ExamineForLoad(vtkStringArray * fileList, vtkCollection * loadables) Line 1988 C++
    vtkSlicerDicomRtImportExportModuleLogicPythonD.dll!PyvtkSlicerDicomRtImportExportModuleLogic_ExamineForLoad(_object * self, _object * args) Line 152    C++

@Sunderlandkyl @lassoan

cpinter commented 4 years ago

I tried with RANDO ENT and PROSTATE, and TinyPatient. It crashed in each case.

The crash does not happen in the nightly of January 6, but it does in that of February 19.

Since the line in the code where the crash happens had a toLatin1 call in it, I quickly applied the UTF-8 changes in case it is related, but it did not improve things. Any help would be appreciated. Without being able to load RT data, SlicerRT is unusable. Thanks.

cpinter commented 4 years ago

I committed a temporary fix so that at least DICOM-RT can be loaded from the application.

The crash, however, happens in tests, for example py_DicomRtImportTest. After loading the structure set, the next concole output is:

12: QSqlQuery::prepare: database not open
12: ASSERT: "!isEmpty()" in file c:\users\qt\work\qt\qtbase\include\qtcore\../../src/corelib/tools/qlist.h, line 345

which is the same place where it crashed in the case above.

cpinter commented 4 years ago

@Sunderlandkyl Since you're leading the SlicerRT maintenance efforts, could you at least react to this please?

Sunderlandkyl commented 4 years ago

Sorry, I didn't see this issue. I'll get right on it. Thanks!

cpinter commented 4 years ago

Thank you! Please make sure you're subscribed to this repository. I usually take care of PRs and minor things, but sometimes it is important that you see these.

For example the recent contributions have broken a few things, which I have partially fixed (there are issues for each). This one is probably not related to those improvements (MLC, ion, etc.) though.

Sunderlandkyl commented 4 years ago

I uncommented the code in https://github.com/SlicerRt/SlicerRT/commit/e089c61f8f23a386694679d61e497b620da984ec, and I can't replicate the same crash.

When I tested, both PROSTATE and TinyPatient both loaded fine, however ENT either crashes during vtkAppendPolyData execution, or never terminates (probably it does, but it may take several hours in debug mode).

The long execution time is a result of a large number of RequestRender calls during load.

Still looking into it.

lassoan commented 4 years ago

This line is a little bit risky:

name += OFString(": ") + OFString(dicomDatabase->fileValue(rtPlanFileName,rtPlanLabelTag).toUtf8().constData());

There are several temporary buffers created, which normally would not be deleted until the expression is evaluated completely, but maybe something is not implemented cleanly. Splitting the expression to several expressions could be safer:

std::string planLabel = dicomDatabase->fileValue(rtPlanFileName,rtPlanLabelTag).toUtf8().constData();
name += OFString(": ") + OFString(planLabel);

It is not very likely that this is an issue, but it is easy to try if it makes a difference.

cpinter commented 4 years ago

I can't replicate the same crash

I have a clean build of Slicer. I'll clean build SlicerRT too and try again.

cpinter commented 4 years ago

I tested this again and now it does not crash. I will uncomment the part that caused a crash in a commit soon.

However, it took around 10 minutes to load the Eclipse ENT test dataset. It only happens if I load the RT plan as well. It seems an unreasonably long time even if the plan is complex. In addition, it created dozens of beam nodes, with one MLC table in each. I added a new ticket for this, see https://github.com/SlicerRt/SlicerRT/issues/145