Thread crashes on multiple clicks



  • 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


  • Lifetime Qt Champion

    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 ?



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


  • Moderators

    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.



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



  • 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(&dummyid);
       patientspecificpslist.push_back(name);
       
      }
      else if(RecType20==MC_REC_TYPE_IMAGE)
      {
       std::string name=getReferencedFileName(directoryFile,dummyid);
       patientspecificxalist.push_back(name);
       MC_Free_File(&dummyid);
      }
      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);

    }
    @

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


  • Lifetime Qt Champion

    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
    }
    @



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


  • Lifetime Qt Champion

    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.



  • Ok thank you ..



  • 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(&dummyid);
       patientspecificpslist.push_back(name);
       
      }
      else if(RecType20==MC_REC_TYPE_IMAGE)
      {
       std::string name=getReferencedFileName(directoryFile,dummyid);
       patientspecificxalist.push_back(name);
       MC_Free_File(&dummyid);
      }
      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.rows
    xars.cols;

    }
    }

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

    @



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


  • Lifetime Qt Champion

    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.


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.