Thread crashes on multiple clicks

ankush205

I have a list view .When i click on list view thumbnails are created .So i start a spread thread to create a thumbnail and keep them posting as hey are created and send those images to main window to display them.Now when i click on the listview item.the thumbania and threads are created properly and icons are displayed properly.Presntly when i click on listview item i terminate a thread if it running.
Now the problem is when i click on one of these item multiple times the thread crashes the application.Also i am allocating memory as a result of which when i call terminate the memory is not deallocated

SGaist

Hi,

Without any code it's pretty much Crystal ball debugging (™) Since you are terminating thread, are you stopping them gracefully or just killing them ?

ankush205

No i am simly terminating..how can i close the thread gracefuly?

JKSH

Hi,

Do not terminate the thread.

[quote author="ankush205" date="1412222464"]No i am simly terminating..how can i close the thread gracefuly?[/quote]From the "QThread documentation":http://qt-project.org/doc/qt-5/qthread.html#terminate about QThread::terminate():

"Warning: This function is dangerous and its use is discouraged. The thread can be terminated at any point in its code path. Threads can be terminated while modifying data. There is no chance for the thread to clean up after itself, unlock any held mutexes, etc. In short, use this function only if absolutely necessary."

Instead of terminating the thread, you need to break out of the loop that is running in the thread, and let the run() function return.

If you'd like more detailed advice, please show us your thread code.

ankush205

Yes i donot have source code at present.Here there is a vacation in India.I would soon post the code

ankush205

This is the code i write for thread and

@void ThumbNailImage::run()
{
getseriesinformation(patientid);

}

void ThumbNailImage::getseriesinformation(int id)
{
MC_STATUS mcStatus;
int studyid=0;
int seriesid=0;
int dummyid=0;
patientspecificpslist.clear();
patientspecificxalist.clear();
MC_DIR_RECORD_TYPE RecType20;
mcStatus= MC_DDH_Get_First_Lower_Record( id, &studyid);
mcStatus= MC_DDH_Get_Record_Type(studyid,&RecType20 );

while(studyid!=0)
{

mcStatus= MC_DDH_Get_First_Lower_Record( studyid, &seriesid);
mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
while(seriesid!=0)
{

 mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
 mcStatus= MC_DDH_Get_First_Lower_Record( seriesid, &dummyid);
 while(dummyid!=0)
 {
  mcStatus= MC_DDH_Get_Record_Type(dummyid,&RecType20 );
  if(RecType20==MC_REC_TYPE_PRESENTATION)
  { 
   
   std::string name=getReferencedFileName(directoryFile,dummyid);
   MC_Free_File&#40;&dummyid&#41;;
   patientspecificpslist.push_back(name&#41;;
   
  }
  else if(RecType20==MC_REC_TYPE_IMAGE)
  {
   std::string name=getReferencedFileName(directoryFile,dummyid);
   patientspecificxalist.push_back(name);
   MC_Free_File&#40;&dummyid&#41;;
  }
  else
  {
  }
  mcStatus= MC_DDH_Get_Next_Record( dummyid, &dummyid &#41;;
  
  
 }

 mcStatus= MC_DDH_Get_Next_Record( seriesid, &seriesid );
    mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
}

mcStatus= MC_DDH_Get_Next_Record( studyid, &studyid );

mcStatus= MC_DDH_Get_Record_Type(studyid,&RecType20 );
}

makepairs(patientspecificpslist,patientspecificxalist);

}
@

In main thread i write
@if(thumbnail->isRunning())
{
thumbnail->releaseMemory();
thumbnail->terminate();
}
thumbnail->initialize(m_appID,id,directoryfile);
thumbnail->start();@

SGaist

You should add a loop breaking variable to allow the loop to end e.g.

@
while(studyid!=0 && continue) {
//do stuff
}

if (continue) {
makepairs(patientspecificpslist,patientspecificxalist);
} else {
// cleanup
}
@

ankush205

Ohk i should not terminate but i should break the for loop

SGaist

Yes, what you are doing is currently using a bazooka to stop your car rather than break. Both ends up with your car stopped but the consequences are not the same.

ankush205

Ok thank you ..

ankush205

After I break the loop some data of earlier thread is posted again.
@void Viewer:: fileselection(QModelIndex ip)
{

QVariant data=tree->data(ip,Qt::DisplayRole);
QString str=data.toString();
std::string s2 = str.toLocal8Bit().constData();
std::wstring wstmp(s2.begin(),s2.end());
if(patientmap.empty())
{
return;
}
int id=patientmap[wstmp];
listwidget->clear();
if(thumbnail->isRunning())
{
thumbnail->StopThread();
thumbnail->quit();
thumbnail->wait(1000);
}

thumbnail->initialize(m_appID,id,directoryfile);
thumbnail->start();

}@

And in the thumbnail class
@

void ThumbNailImage::run()
{

if(!stop==true)
{
getseriesinformation(patientid);
}
}

void ThumbNailImage::getseriesinformation(int id)
{
MC_STATUS mcStatus;
int studyid=0;
int seriesid=0;
int dummyid=0;
patientspecificpslist.clear();
patientspecificxalist.clear();
MC_DIR_RECORD_TYPE RecType20;
mcStatus= MC_DDH_Get_First_Lower_Record( id, &studyid);
mcStatus= MC_DDH_Get_Record_Type(studyid,&RecType20 );

while(studyid!=0)
{

mcStatus= MC_DDH_Get_First_Lower_Record( studyid, &seriesid);
mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
while(seriesid!=0)
{

 mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
 mcStatus= MC_DDH_Get_First_Lower_Record( seriesid, &dummyid);
 while(dummyid!=0)
 {
  mcStatus= MC_DDH_Get_Record_Type(dummyid,&RecType20 );
  if(RecType20==MC_REC_TYPE_PRESENTATION)
  { 
   
   std::string name=getReferencedFileName(directoryFile,dummyid);
   MC_Free_File&#40;&dummyid&#41;;
   patientspecificpslist.push_back(name);
   
  }
  else if(RecType20==MC_REC_TYPE_IMAGE)
  {
   std::string name=getReferencedFileName(directoryFile,dummyid);
   patientspecificxalist.push_back(name);
   MC_Free_File&#40;&dummyid&#41;;
  }
  else
  {
  }
  mcStatus= MC_DDH_Get_Next_Record( dummyid, &dummyid );
  
  
 }

 mcStatus= MC_DDH_Get_Next_Record( seriesid, &seriesid );
    mcStatus= MC_DDH_Get_Record_Type(seriesid,&RecType20 );
}

mcStatus= MC_DDH_Get_Next_Record( studyid, &studyid );

mcStatus= MC_DDH_Get_Record_Type(studyid,&RecType20 );
}

makepairs(patientspecificpslist,patientspecificxalist);

}

void ThumbNailImage::makepairs(vector<string> pslist,vector<string> xalist)
{

bool checkflag=false;
int xaid=0;
int psid;
std::string pssopid;
std::string xasopid;

if(stop==false)
{
for(auto &i: xalist)
{

bool bStatus=true;
xaid=readDCMFile(i.c_str(),m_appID);
bStatus = bStatus && getAttribute(Type1, MC_ATT_SOP_INSTANCE_UID,xaid,xasopid,1024);

for ( auto &j : pslist )
{
psid=readDCMFile(j.c_str(),m_appID);
int seriesid;
int imageid;
bStatus = bStatus && getAttribute(Type1,MC_ATT_REFERENCED_SERIES_SEQUENCE,psid,seriesid);
bStatus = bStatus && getAttribute(Type1,MC_ATT_REFERENCED_IMAGE_SEQUENCE,seriesid,imageid);
bStatus = bStatus && getAttribute(Type1,MC_ATT_REFERENCED_SOP_INSTANCE_UID,imageid,pssopid,1024);

if((strcmp(xasopid.c_str(),pssopid.c_str())==0))
{
xapsmap.insert( std::make_pair( i,j));
pslist.erase(std::remove(pslist.begin(), pslist.end(),j), pslist.end());
 createThumbNail(xaid);

MC_Free_File&#40;&psid&#41;;
checkflag=true;
break;
}

 MC_Free_File&#40;&psid&#41;;

}

MC_Free_File(&xaid);
}
}
}
void ThumbNailImage::createThumbNail(int id)

{

if(stop==false)
{
XAReader xar(id);
struct xadata xars;
struct psdata psrs;
xar.readAttributes(xars);

if(processor!=nullptr)
delete processor;
processor=new ImageProcessor(id,0,&env);

processor->setfileinfo(xars,psrs);
if(buffer!=nullptr)
delete[] buffer;
buffer=new unsigned char[xars.rows*xars.cols];
processor->createThumbnail(buffer);
QRgb valuegray;
if(thumbimage!=nullptr)
delete thumbimage;
thumbimage=new QImage(xars.rows,xars.cols, QImage::Format_RGB32);
for(int i=0;i<xars.rows;i++)
{
for(int j=0;j<xars.cols;j++)
{
valuegray=qRgb(*buffer,*buffer,*buffer);
thumbimage->setPixel(j,i,valuegray);
buffer++;

}
}
thumbimage=thumbimage->scaled(80,80,Qt::IgnoreAspectRatio,Qt::FastTransformation);
emit msend(thumbimage);
buffer=buffer-xars.rowsxars.cols;

}
}

void ThumbNailImage::StopThread()
{
stop=true;
xapsmap.clear();
patientspecificpslist.clear();
patientspecificxalist.clear();
}

@

ankush205

Everything works well when the operation is slowly performed..bt on rapid switching wrong data is obtained

SGaist

Reacting on something like currentItemChanged ? You should buffer that using e.g. a QTimer. Stop/Starting a new batch of operation while the user still moves around the list is a bad idea. You can stop the thread if it's already running but don't restart a new one right after since the user might be still moving around.