[bt-devel] Crash While Canceling Bookshelf Download

Gary Holmlund gary.holmlund at gmail.com
Thu Nov 5 22:29:16 MST 2009


I found the cause of the crash when canceling bookshelf downloads. 
Before calling the slotStopInstall function the m_iMgr (BtInstallMgr) is 
receiving regular calls from sword that cause the progress bar to 
update. In the slotStopInstall function the m_iMgr->terminate() tries to 
stop the thread, but this is not guaranteed.  Then there is a wait for 
200 ms hoping the thread terminates. If it is not stopped the terminate 
is used again and a wait of 2 ms is done. After this the m_iMgr is 
deleted. Unfortunately the thread is not yet done and calls m_iMgr again 
for a progress bar update. This crashes since m_iMgr was deleted.

void BtInstallThread::slotStopInstall() {
    qDebug() << "*****************\nBtInstallThread::slotStopInstall" << 
m_module << "\n********";
    if (!done) {
        done = true;
        qDebug() << "**********\nBtInstallThread::slotStopInstall, 
installing" << m_module << "was cancelled\n*******";
        m_iMgr->terminate();
        //this->terminate(); // It's dangerous to forcibly stop, but we 
will clean up the files
        qDebug() << "BtInstallThread::slotStopInstall 2";
        //qApp->processEvents();
        // wait to terminate for some secs. We rather let the execution 
go on and cleaning up to fail than the app to freeze
        int notRun = this->wait(200);
        if (notRun) {
            this->terminate();
            this->wait(2);
            qDebug() << "installthread (" << m_module << ") terminated, 
delete m_iMgr";
            delete m_iMgr; // this makes sure the ftp library will be 
cleaned up in the destroyer
            m_iMgr = 0;
        }

The wait function will return as soon as the thread does terminate so 
there is not much cost to greatly increasing the time. I put 25,000 ms 
on the first wait and 10,000 on the second. I have not seen a crash 
since then. I did see from 0 to 5 second delay after canceling before 
the dialog went away. We would have to have a 35 second delay before a 
crash could still occur.

I have checked in this change.

Gary





More information about the bt-devel mailing list