From 7dc822e55421288a0c8a67ea8e85df5c5e50dace Mon Sep 17 00:00:00 2001 From: Timothy Pearson Date: Sat, 2 Nov 2013 23:06:22 -0500 Subject: Remove botched transfer slave threading code This largely resolves Bug 1670 --- kget/common.h | 1 - kget/main.cpp | 5 ++--- kget/slave.cpp | 61 +++++++++++--------------------------------------- kget/slave.h | 1 + kget/tdemainwidget.cpp | 8 +++---- kget/transfer.cpp | 56 +++++++++++++++++++++++++++++++++++---------- kget/transfer.h | 6 +++-- 7 files changed, 68 insertions(+), 70 deletions(-) diff --git a/kget/common.h b/kget/common.h index b436902a..4b3606a8 100644 --- a/kget/common.h +++ b/kget/common.h @@ -21,7 +21,6 @@ #ifndef _COMMON_H #define _COMMON_H - #include #define mDebugIn mDebug << ">>>>Entering " diff --git a/kget/main.cpp b/kget/main.cpp index 1c1b9eb5..7f65e3f5 100644 --- a/kget/main.cpp +++ b/kget/main.cpp @@ -24,7 +24,6 @@ * ***************************************************************************/ - #include #include #include @@ -102,7 +101,7 @@ class KGetApp : public KUniqueApplication { private: TDEMainWidget *tdemainwidget; - + public: KGetApp() : KUniqueApplication() { @@ -171,7 +170,7 @@ public: for( int i=0; i < args->count(); ++i){ urls.append(KURL::fromPathOrURL( args->arg(i))); } - + // Sometimes valid filenames are not recognised by KURL::isLocalFile(), they are marked as invalid then if ( args->count()==2 && ( urls.last().isLocalFile() || !urls.last().isValid())) { diff --git a/kget/slave.cpp b/kget/slave.cpp index 9aa91810..60933dcd 100644 --- a/kget/slave.cpp +++ b/kget/slave.cpp @@ -24,9 +24,7 @@ * ***************************************************************************/ - #include -#include #include "getfilejob.h" #include "slave.h" @@ -39,6 +37,10 @@ Slave::Slave(Transfer * _parent, const KURL & _src, const KURL & _dest) : TQObject(), TQThread() { + // FIXME + // KGet uses an unconventional threading model that relies on TDEIO slave execution from the main GUI thread + disableThreadPostedEvents(true); + mDebug << ">>>>Entering" << endl; copyjob = NULL; m_src = _src; @@ -57,8 +59,10 @@ void Slave::Op(SlaveCommand _cmd) { mDebugIn << " _cmd = " << _cmd << endl; - if ( !running() ) // start on demand + if ( !running() ) { // start on demand start(); + moveToThread(this); + } mutex.lock(); stack.push(_cmd); @@ -93,8 +97,6 @@ void Slave::InfoMessage(const TQString & _msg) mDebug << "Infor Msg:" << "_msg = " << _msg << endl; } - - void Slave::run() { mDebugIn << endl; @@ -102,89 +104,53 @@ void Slave::run() SlaveCommand cmd; bool running = true; - while (running) + while (running) { - if (!nPendingCommand) + if (!nPendingCommand) { worker.wait(); - switch (cmd = fetch_cmd()) + } + switch (cmd = fetch_cmd()) { case RESTART: - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } // fall through case RETR: mDebug << " FETCHED COMMAND RETR" << endl; - assert(!copyjob); - TDEIO::Scheduler::checkSlaveOnHold( true ); - copyjob = new TDEIO::GetFileJob(m_src, m_dest); - Connect(); PostMessage(SLV_RESUMED); break; case RETR_CACHE: mDebug << " FETCHED COMMAND RETR_CACHE" << endl; - assert(!copyjob); - TDEIO::Scheduler::checkSlaveOnHold( true ); - copyjob = new TDEIO::GetFileJob(m_src, m_dest); - copyjob->addMetaData("cache", "cacheonly"); - Connect(); break; case PAUSE: mDebug << " FETCHED COMMAND PAUSE" << endl; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } PostMessage(SLV_PAUSED); break; case KILL: mDebug << " FETCHED COMMAND KILL" << endl; running = false; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } // no message posted break; - + case REMOVE: mDebug << " FETCHED COMMAND REMOVE" << endl; running = false; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } PostMessage(SLV_REMOVED); break; case SCHEDULE: mDebug << " FETCHED COMMAND SCHEDULE" << endl; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } PostMessage(SLV_SCHEDULED); break; case DELAY: mDebug << " FETCHED COMMAND DELAY" << endl; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } PostMessage(SLV_DELAYED); break; case NOOP: mDebug << "FETCHED COMMAND NOOP, i.e. empty stack" << endl; - if (copyjob) { - copyjob->kill(true); - copyjob = 0L; - } running = false; break; @@ -217,7 +183,6 @@ void Slave::Connect() { mDebugIn << endl; - connect(copyjob, TQT_SIGNAL(canceled(TDEIO::Job *)), TQT_SLOT(slotCanceled(TDEIO::Job *))); connect(copyjob, TQT_SIGNAL(connected(TDEIO::Job *)), TQT_SLOT(slotConnected(TDEIO::Job *))); connect(copyjob, TQT_SIGNAL(result(TDEIO::Job *)), TQT_SLOT(slotResult(TDEIO::Job *))); @@ -253,7 +218,7 @@ void Slave::slotConnected(TDEIO::Job *) void Slave::slotResult(TDEIO::Job * job) { mDebugIn << endl; - + assert(copyjob == job); copyjob=0L; TDEIO::Error error=TDEIO::Error(job->error()); diff --git a/kget/slave.h b/kget/slave.h index cca9df86..f40283ad 100644 --- a/kget/slave.h +++ b/kget/slave.h @@ -100,6 +100,7 @@ private: TQMutex mutex; TDEIO::GetFileJob * copyjob; + friend class Transfer; }; #endif diff --git a/kget/tdemainwidget.cpp b/kget/tdemainwidget.cpp index a7d51617..2f7e0a26 100644 --- a/kget/tdemainwidget.cpp +++ b/kget/tdemainwidget.cpp @@ -1412,15 +1412,13 @@ void TDEMainWidget::slotAnimTimeout() //sDebugIn << endl; #endif - bool isTransfer; - animCounter++; if (animCounter == myTransferList->getPhasesNum()) { //updateStatusBar(); animCounter = 0; } // update status of all items of transferList - isTransfer = myTransferList->updateStatus(animCounter); + myTransferList->updateStatus(animCounter); //if (this->isVisible()) { updateStatusBar(); @@ -2221,7 +2219,9 @@ void TDEMainWidget::onlineDisconnect() } } log(i18n("Disconnecting...")); - system(TQFile::encodeName(ksettings.disconnectCommand)); + if (system(TQFile::encodeName(ksettings.disconnectCommand)) < 0) { + // Error! + } #ifdef _DEBUG sDebugOut << endl; diff --git a/kget/transfer.cpp b/kget/transfer.cpp index d1ff6f0b..f8ef34f3 100644 --- a/kget/transfer.cpp +++ b/kget/transfer.cpp @@ -24,6 +24,7 @@ ***************************************************************************/ #include +#include #include #include @@ -43,11 +44,13 @@ #include "dlgIndividual.h" #include "transferlist.h" #include "transfer.h" +#include "getfilejob.h" #include #include #include #include +#include extern Settings ksettings; @@ -117,7 +120,6 @@ Transfer::init(const uint _id) status = ST_STOPPED; - connect(this, TQT_SIGNAL(statusChanged(Transfer *, int)), tdemain, TQT_SLOT(slotStatusChanged(Transfer *, int))); connect(this, TQT_SIGNAL(statusChanged(Transfer *, int)), this, TQT_SLOT(slotUpdateActions())); @@ -160,6 +162,7 @@ void Transfer::synchronousAbort() { if ( m_pSlave->running() ) { + destroyGetFileJob(); m_pSlave->Op(Slave::KILL); m_pSlave->wait(); } @@ -281,14 +284,13 @@ void Transfer::updateAll() // destination setText(view->lv_filename, dest.fileName()); - if(dlgIndividual) - { - dlgIndividual->setCopying(src, dest); - dlgIndividual->setCanResume(canResume); - dlgIndividual->setTotalSize(totalSize); - dlgIndividual->setPercent(0); - dlgIndividual->setProcessedSize(0); - } + if (dlgIndividual) { + dlgIndividual->setCopying(src, dest); + dlgIndividual->setCanResume(canResume); + dlgIndividual->setTotalSize(totalSize); + dlgIndividual->setPercent(0); + dlgIndividual->setProcessedSize(0); + } if (totalSize != 0) { //logMessage(i18n("Total size is %1 bytes").arg((double)totalSize)); @@ -389,6 +391,7 @@ void Transfer::slotResume() logMessage(i18n("Attempt number %1").arg(retryCount)); sDebug << "sending Resume to slave " << endl; + createGetFileJob(m_pSlave->m_src, m_pSlave->m_dest, false); m_pSlave->Op(Slave::RETR); sDebugOut << endl; @@ -403,6 +406,7 @@ void Transfer::slotResume() assert(status <= ST_RUNNING && ksettings.b_offline); + destroyGetFileJob(); m_pSlave->Op(Slave::KILL); // KILL doesn't post a Message sDebug << "Killing Slave" << endl; @@ -430,7 +434,7 @@ void Transfer::slotRequestPause() m_paPause->setEnabled(false); m_paRestart->setEnabled(false); - + destroyGetFileJob(); m_pSlave->Op(Slave::PAUSE); sDebug << "Requesting Pause.." << endl; @@ -443,6 +447,8 @@ void Transfer::slotRequestPause() void Transfer::slotRequestRestart() { sDebugIn << endl; + destroyGetFileJob(); + createGetFileJob(m_pSlave->m_src, m_pSlave->m_dest, true); m_pSlave->Op(Slave::RESTART); slotSpeed(0); sDebugOut << endl; @@ -477,10 +483,13 @@ void Transfer::slotRequestRemove() SafeDelete::deleteFile( file ); // ### messagebox on failure? } } - if (status == ST_RUNNING) + if (status == ST_RUNNING) { + destroyGetFileJob(); m_pSlave->Op(Slave::REMOVE); - else + } + else { emit statusChanged(this, OP_REMOVED); + } sDebugOut << endl; } @@ -517,6 +526,7 @@ void Transfer::slotRequestSchedule() if (status == ST_RUNNING) { m_paPause->setEnabled(false); m_paRestart->setEnabled(false); + destroyGetFileJob(); m_pSlave->Op(Slave::SCHEDULE); } else @@ -535,6 +545,7 @@ void Transfer::slotRequestDelay() if (status == ST_RUNNING) { m_paPause->setEnabled(false); m_paRestart->setEnabled(false); + destroyGetFileJob(); m_pSlave->Op(Slave::DELAY); } else slotExecDelay(); @@ -894,6 +905,7 @@ void Transfer::slotExecConnected() if (ksettings.b_offline)// when we're offline and arrive here, then the file is in cache return; // Slave::slotResult will be called immediately, so we do nothing here status = ST_STOPPED; + destroyGetFileJob(); m_pSlave->Op(Slave::KILL); if (ksettings.b_addQueued) { @@ -1006,6 +1018,7 @@ void Transfer::checkCache() if (src.protocol()=="http") { status = ST_TRYING; + createGetFileJob(m_pSlave->m_src, m_pSlave->m_dest, true); m_pSlave->Op(Slave::RETR_CACHE); } else @@ -1021,5 +1034,24 @@ void Transfer::NotInCache() mode = MD_DELAYED; status = ST_STOPPED; } + +void Transfer::destroyGetFileJob() +{ + if (m_pSlave->copyjob) { + m_pSlave->copyjob->kill(true); + m_pSlave->copyjob = 0L; + } +} + +void Transfer::createGetFileJob(KURL m_src, KURL m_dest, bool cache) +{ + TDEIO::Scheduler::checkSlaveOnHold( true ); + m_pSlave->copyjob = new TDEIO::GetFileJob(m_src, m_dest); + if (cache) { + m_pSlave->copyjob->addMetaData("cache", "cacheonly"); + } + m_pSlave->Connect(); +} + #include "transfer.moc" diff --git a/kget/transfer.h b/kget/transfer.h index 17dd24be..4a05b49b 100644 --- a/kget/transfer.h +++ b/kget/transfer.h @@ -188,13 +188,15 @@ public slots: void slotStartTime(const TQDateTime &); void slotStop(); // stop all transfers when going offline - + signals: void statusChanged(Transfer *, int _operation); void log(uint, const TQString &, const TQString &); private: void init(const uint _id); + void destroyGetFileJob(); + void createGetFileJob(KURL, KURL, bool); Slave *m_pSlave; @@ -208,7 +210,7 @@ private: // log TQString transferLog; - + // schedule time TQDateTime startTime; -- cgit v1.2.1