From 41babc2d252ef8c0c63ac75a182536a9c754b3bc Mon Sep 17 00:00:00 2001 From: Ivailo Monev Date: Thu, 2 Jun 2022 16:19:17 +0300 Subject: [PATCH] kio: remove unused KIO::Scheduler methods I expect some race-conditions to manifest from such changes Signed-off-by: Ivailo Monev --- kio/kio/scheduler.cpp | 213 +------------------------------------ kio/kio/scheduler.h | 75 ++----------- kio/kio/scheduler_p.h | 35 ------ kio/tests/kioslavetest.cpp | 19 ++-- kio/tests/kioslavetest.h | 3 - 5 files changed, 20 insertions(+), 325 deletions(-) diff --git a/kio/kio/scheduler.cpp b/kio/kio/scheduler.cpp index 2bb0e4e3..7b81cd15 100644 --- a/kio/kio/scheduler.cpp +++ b/kio/kio/scheduler.cpp @@ -254,147 +254,6 @@ QList HostQueue::allSlaves() const return ret; } - - -ConnectedSlaveQueue::ConnectedSlaveQueue() -{ - m_startJobsTimer.setSingleShot(true); - connect (&m_startJobsTimer, SIGNAL(timeout()), SLOT(startRunnableJobs())); -} - -bool ConnectedSlaveQueue::queueJob(SimpleJob *job, Slave *slave) -{ - QHash::Iterator it = m_connectedSlaves.find(slave); - if (it == m_connectedSlaves.end()) { - return false; - } - SimpleJobPrivate::get(job)->m_slave = slave; - - PerSlaveQueue &jobs = it.value(); - jobs.waitingList.append(job); - if (!jobs.runningJob) { - // idle slave now has a job to run - m_runnableSlaves.insert(slave); - m_startJobsTimer.start(); - } - return true; -} - -bool ConnectedSlaveQueue::removeJob(SimpleJob *job) -{ - Slave *slave = jobSlave(job); - Q_ASSERT(slave); - QHash::Iterator it = m_connectedSlaves.find(slave); - if (it == m_connectedSlaves.end()) { - return false; - } - PerSlaveQueue &jobs = it.value(); - if (jobs.runningJob || jobs.waitingList.isEmpty()) { - // a slave that was busy running a job was not runnable. - // a slave that has no waiting job(s) was not runnable either. - Q_ASSERT(!m_runnableSlaves.contains(slave)); - } - - const bool removedRunning = jobs.runningJob == job; - const bool removedWaiting = jobs.waitingList.removeAll(job) != 0; - if (removedRunning) { - jobs.runningJob = 0; - Q_ASSERT(!removedWaiting); - } - const bool removedTheJob = removedRunning || removedWaiting; - - if (!slave->isAlive()) { - removeSlave(slave); - return removedTheJob; - } - - if (removedRunning && jobs.waitingList.count()) { - m_runnableSlaves.insert(slave); - m_startJobsTimer.start(); - } - if (removedWaiting && jobs.waitingList.isEmpty()) { - m_runnableSlaves.remove(slave); - } - return removedTheJob; -} - -void ConnectedSlaveQueue::addSlave(Slave *slave) -{ - Q_ASSERT(slave); - if (!m_connectedSlaves.contains(slave)) { - m_connectedSlaves.insert(slave, PerSlaveQueue()); - } -} - -bool ConnectedSlaveQueue::removeSlave(Slave *slave) -{ - QHash::Iterator it = m_connectedSlaves.find(slave); - if (it == m_connectedSlaves.end()) { - return false; - } - PerSlaveQueue &jobs = it.value(); - Q_FOREACH (SimpleJob *job, jobs.waitingList) { - // ### for compatibility with the old scheduler we don't touch the running job, if any. - // make sure that the job doesn't call back into Scheduler::cancelJob(); this would - // a) crash and b) be unnecessary because we clean up just fine. - SimpleJobPrivate::get(job)->m_schedSerial = 0; - job->kill(); - } - m_connectedSlaves.erase(it); - m_runnableSlaves.remove(slave); - - slave->kill(); - return true; -} - -// KDE5: only one caller, for doubtful reasons. remove this if possible. -bool ConnectedSlaveQueue::isIdle(Slave *slave) -{ - QHash::Iterator it = m_connectedSlaves.find(slave); - if (it == m_connectedSlaves.end()) { - return false; - } - return it.value().runningJob == 0; -} - - -//private slot -void ConnectedSlaveQueue::startRunnableJobs() -{ - QSet::Iterator it = m_runnableSlaves.begin(); - while (it != m_runnableSlaves.end()) { - Slave *slave = *it; - if (!slave->isConnected()) { - // this polling is somewhat inefficient... - m_startJobsTimer.start(); - ++it; - continue; - } - it = m_runnableSlaves.erase(it); - PerSlaveQueue &jobs = m_connectedSlaves[slave]; - SimpleJob *job = jobs.waitingList.takeFirst(); - Q_ASSERT(!jobs.runningJob); - jobs.runningJob = job; - - const KUrl url = job->url(); - // no port is -1 in QUrl, but in kde3 we used 0 and the kioslaves assume that. - const int port = url.port() == -1 ? 0 : url.port(); - - if (slave->host() == "") { - MetaData configData = SlaveConfig::self()->configData(url.protocol(), url.host()); - slave->setConfig(configData); - slave->setProtocol(url.protocol()); - slave->setHost(url.host(), port, url.user(), url.pass()); - } - - Q_ASSERT(slave->protocol() == url.protocol()); - Q_ASSERT(slave->host() == url.host()); - Q_ASSERT(slave->port() == port); - startJob(job, slave); - } -} - - static void ensureNoDuplicates(QMap *queuesBySerial) { Q_UNUSED(queuesBySerial); @@ -561,9 +420,7 @@ void ProtoQueue::removeJob(SimpleJob *job) // should be a connected slave // if the assertion fails the job has probably changed the host part of its URL while // running, so we can't find it by hostname. don't do this. - const bool removed = m_connectedSlaveQueue.removeJob(job); - Q_UNUSED(removed); - Q_ASSERT(removed); + Q_ASSERT(false); } ensureNoDuplicates(&m_queuesBySerial); @@ -590,10 +447,9 @@ Slave *ProtoQueue::createSlave(const QString &protocol, SimpleJob *job, const KU bool ProtoQueue::removeSlave (KIO::Slave *slave) { - const bool removedConnected = m_connectedSlaveQueue.removeSlave(slave); const bool removedUnconnected = m_slaveKeeper.removeSlave(slave); - Q_ASSERT(!(removedConnected && removedUnconnected)); - return removedConnected || removedUnconnected; + Q_ASSERT(!removedUnconnected); + return removedUnconnected; } QList ProtoQueue::allSlaves() const @@ -602,7 +458,6 @@ QList ProtoQueue::allSlaves() const Q_FOREACH (const HostQueue &hq, m_queuesByHostname) { ret.append(hq.allSlaves()); } - ret.append(m_connectedSlaveQueue.allSlaves()); return ret; } @@ -728,9 +583,6 @@ public: void jobFinished(KIO::SimpleJob *job, KIO::Slave *slave); void putSlaveOnHold(KIO::SimpleJob *job, const KUrl &url); void removeSlaveOnHold(); - Slave *getConnectedSlave(const KUrl &url, const KIO::MetaData &metaData); - bool assignJobToSlave(KIO::Slave *slave, KIO::SimpleJob *job); - bool disconnectSlave(KIO::Slave *slave); void checkSlaveOnHold(bool b); void publishSlaveOnHold(); Slave *heldSlaveForJob(KIO::SimpleJob *job); @@ -871,22 +723,6 @@ void Scheduler::updateInternalMetaData(SimpleJob* job) schedulerPrivate->updateInternalMetaData(job); } -KIO::Slave *Scheduler::getConnectedSlave(const KUrl &url, - const KIO::MetaData &config ) -{ - return schedulerPrivate->getConnectedSlave(url, config); -} - -bool Scheduler::assignJobToSlave(KIO::Slave *slave, KIO::SimpleJob *job) -{ - return schedulerPrivate->assignJobToSlave(slave, job); -} - -bool Scheduler::disconnectSlave(KIO::Slave *slave) -{ - return schedulerPrivate->disconnectSlave(slave); -} - void Scheduler::registerWindow(QWidget *wid) { schedulerPrivate->registerWindow(wid); @@ -1230,28 +1066,6 @@ void SchedulerPrivate::removeSlaveOnHold() m_urlOnHold.clear(); } -Slave *SchedulerPrivate::getConnectedSlave(const KUrl &url, const KIO::MetaData &config) -{ - QStringList proxyList; - const QString protocol = KProtocolManager::slaveProtocol(url, proxyList); - ProtoQueue *pq = protoQ(protocol, url.host()); - - Slave *slave = pq->createSlave(protocol, /* job */0, url); - if (slave) { - setupSlave(slave, url, protocol, proxyList, true, &config); - pq->m_connectedSlaveQueue.addSlave(slave); - - slave->send( CMD_CONNECT ); - q->connect(slave, SIGNAL(connected()), - SLOT(slotSlaveConnected())); - q->connect(slave, SIGNAL(error(int,QString)), - SLOT(slotSlaveError(int,QString))); - } - kDebug(7006) << url << slave; - return slave; -} - - void SchedulerPrivate::slotSlaveConnected() { kDebug(7006); @@ -1266,32 +1080,13 @@ void SchedulerPrivate::slotSlaveError(int errorNr, const QString &errorMsg) Slave *slave = static_cast(q->sender()); kDebug(7006) << slave << errorNr << errorMsg; ProtoQueue *pq = protoQ(slave->protocol(), slave->host()); - if (!slave->isConnected() || pq->m_connectedSlaveQueue.isIdle(slave)) { + if (!slave->isConnected()) { // Only forward to application if slave is idle or still connecting. // ### KDE5: can we remove this apparently arbitrary behavior and just always emit SlaveError? emit q->slaveError(slave, errorNr, errorMsg); } } -bool SchedulerPrivate::assignJobToSlave(KIO::Slave *slave, SimpleJob *job) -{ - kDebug(7006) << slave << job; - // KDE5: queueing of jobs can probably be removed, it provides very little benefit - ProtoQueue *pq = m_protocols.value(slave->protocol()); - if (pq) { - pq->removeJob(job); - return pq->m_connectedSlaveQueue.queueJob(job, slave); - } - return false; -} - -bool SchedulerPrivate::disconnectSlave(KIO::Slave *slave) -{ - kDebug(7006) << slave; - ProtoQueue *pq = m_protocols.value(slave->protocol()); - return (pq ? pq->m_connectedSlaveQueue.removeSlave(slave) : false); -} - void SchedulerPrivate::checkSlaveOnHold(bool b) { kDebug(7006) << b; diff --git a/kio/kio/scheduler.h b/kio/kio/scheduler.h index b35c8ea2..e07a1485 100644 --- a/kio/kio/scheduler.h +++ b/kio/kio/scheduler.h @@ -78,28 +78,11 @@ namespace KIO { * * Example: * \code - * Slave *slave = KIO::Scheduler::getConnectedSlave( - * KUrl("pop3://bastian:password@mail.kde.org")); * TransferJob *job1 = KIO::get( * KUrl("pop3://bastian:password@mail.kde.org/msg1")); - * KIO::Scheduler::assignJobToSlave(slave, job1); - * TransferJob *job2 = KIO::get( - * KUrl("pop3://bastian:password@mail.kde.org/msg2")); - * KIO::Scheduler::assignJobToSlave(slave, job2); - * TransferJob *job3 = KIO::get( - * KUrl("pop3://bastian:password@mail.kde.org/msg3")); - * KIO::Scheduler::assignJobToSlave(slave, job3); + * KIO::Scheduler::doJob(job1); * * // ... Wait for jobs to finish... - * - * KIO::Scheduler::disconnectSlave(slave); - * \endcode - * - * Note that you need to explicitly disconnect the slave when the - * connection goes down, so your error handler should contain: - * \code - * if (error == KIO::ERR_CONNECTION_BROKEN) - * KIO::Scheduler::disconnectSlave(slave); * \endcode * * @see KIO::Slave @@ -117,7 +100,7 @@ namespace KIO { * is available. This can be changed by calling setJobPriority. * @param job the job to register */ - static void doJob(SimpleJob *job); + static void doJob(KIO::SimpleJob *job); /** * Changes the priority of @p job; jobs of the same priority run in the order in which @@ -127,13 +110,13 @@ namespace KIO { * @param job the job to change * @param priority new priority of @p job, lower runs earlier */ - static void setJobPriority(SimpleJob *job, int priority); + static void setJobPriority(KIO::SimpleJob *job, int priority); /** * Stop the execution of a job. * @param job the job to cancel */ - static void cancelJob(SimpleJob *job); + static void cancelJob(KIO::SimpleJob *job); /** * Called when a job is done. @@ -168,52 +151,6 @@ namespace KIO { */ static void publishSlaveOnHold(); - /** - * Requests a slave for use in connection-oriented mode. - * - * @param url This defines the username,password,host & port to - * connect with. - * @param config Configuration data for the slave. - * - * @return A pointer to a connected slave or 0 if an error occurred. - * @see assignJobToSlave() - * @see disconnectSlave() - */ - static KIO::Slave *getConnectedSlave(const KUrl &url, - const KIO::MetaData &config = MetaData() ); - - /** - * Uses @p slave to do @p job. - * This function should be called immediately after creating a Job. - * - * @param slave The slave to use. The slave must have been obtained - * with a call to getConnectedSlave and must not - * be currently assigned to any other job. - * @param job The job to do. - * - * @return true is successful, false otherwise. - * - * @see getConnectedSlave() - * @see disconnectSlave() - * @see slaveConnected() - * @see slaveError() - */ - static bool assignJobToSlave(KIO::Slave *slave, KIO::SimpleJob *job); - - /** - * Disconnects @p slave. - * - * @param slave The slave to disconnect. The slave must have been - * obtained with a call to getConnectedSlave - * and must not be assigned to any job. - * - * @return true is successful, false otherwise. - * - * @see getConnectedSlave - * @see assignJobToSlave - */ - static bool disconnectSlave(KIO::Slave *slave); - /** * Register the mainwindow @p wid with the KIO subsystem * Do not call this, it is called automatically from @@ -251,6 +188,8 @@ namespace KIO { */ static void updateInternalMetaData(SimpleJob* job); + static Scheduler *self(); + Q_SIGNALS: void slaveConnected(KIO::Slave *slave); void slaveError(KIO::Slave *slave, int error, const QString &errorMsg); @@ -264,8 +203,6 @@ namespace KIO { Scheduler(); ~Scheduler(); - static Scheduler *self(); - Q_PRIVATE_SLOT(d_func(), void slotSlaveDied(KIO::Slave *slave)) Q_PRIVATE_SLOT(d_func(), void slotSlaveStatus(pid_t pid, const QByteArray &protocol, const QString &host, bool connected)) diff --git a/kio/kio/scheduler_p.h b/kio/kio/scheduler_p.h index 8fd723b9..237b66b1 100644 --- a/kio/kio/scheduler_p.h +++ b/kio/kio/scheduler_p.h @@ -87,40 +87,6 @@ private: QSet m_runningJobs; }; -struct PerSlaveQueue -{ - PerSlaveQueue() : runningJob(0) {} - QList waitingList; - SimpleJob *runningJob; -}; - -class ConnectedSlaveQueue : public QObject -{ - Q_OBJECT -public: - ConnectedSlaveQueue(); - - bool queueJob(KIO::SimpleJob *job, KIO::Slave *slave); - bool removeJob(KIO::SimpleJob *job); - - void addSlave(KIO::Slave *slave); - bool removeSlave(KIO::Slave *slave); - - // KDE5: only one caller, for doubtful reasons. remove this if possible. - bool isIdle(KIO::Slave *slave); - bool isEmpty() const { return m_connectedSlaves.isEmpty(); } - QList allSlaves() const { return m_connectedSlaves.keys(); } - -private slots: - void startRunnableJobs(); -private: - // note that connected slaves stay here when idle, they are not returned to SlaveKeeper - QHash m_connectedSlaves; - QSet m_runnableSlaves; - QTimer m_startJobsTimer; -}; - - class SerialPicker { public: @@ -159,7 +125,6 @@ public: KIO::Slave *createSlave(const QString &protocol, KIO::SimpleJob *job, const KUrl &url); bool removeSlave (KIO::Slave *slave); QList allSlaves() const; - ConnectedSlaveQueue m_connectedSlaveQueue; private slots: // start max one (non-connected) job and return diff --git a/kio/tests/kioslavetest.cpp b/kio/tests/kioslavetest.cpp index 2e27bf79..45804bdd 100644 --- a/kio/tests/kioslavetest.cpp +++ b/kio/tests/kioslavetest.cpp @@ -172,12 +172,15 @@ KioslaveTest::KioslaveTest( QString src, QString dest, uint op, uint pr ) main_widget->setMinimumSize( main_widget->sizeHint() ); setCentralWidget( main_widget ); - slave = 0; -// slave = KIO::Scheduler::getConnectedSlave(KUrl("ftp://ftp.gnu.org/")); - KIO::Scheduler::connect(SIGNAL(slaveConnected(KIO::Slave*)), - this, SLOT(slotSlaveConnected())); - KIO::Scheduler::connect(SIGNAL(slaveError(KIO::Slave*,int,QString)), - this, SLOT(slotSlaveError())); + connect( + KIO::Scheduler::self(), + SIGNAL(slaveConnected(KIO::Slave*)), + this, SLOT(slotSlaveConnected()) + ); + connect( + KIO::Scheduler::self(), SIGNAL(slaveError(KIO::Slave*,int,QString)), + this, SLOT(slotSlaveError()) + ); } void KioslaveTest::slotQuit(){ @@ -291,9 +294,8 @@ void KioslaveTest::startJob() { } if (myJob) { - if (slave) - KIO::Scheduler::assignJobToSlave(slave, myJob); job = myJob; + KIO::Scheduler::doJob(myJob); } statusBar()->addWidget( statusTracker->widget(job), 0 ); @@ -345,7 +347,6 @@ void KioslaveTest::slotSlaveConnected() void KioslaveTest::slotSlaveError() { kDebug() << "Error connected."; - slave = 0; } void KioslaveTest::printUDSEntry( const KIO::UDSEntry & entry ) diff --git a/kio/tests/kioslavetest.h b/kio/tests/kioslavetest.h index 7de84b6c..bb1602dc 100644 --- a/kio/tests/kioslavetest.h +++ b/kio/tests/kioslavetest.h @@ -36,8 +36,6 @@ public: if ( job ) { job->kill( KJob::Quietly ); // kill the job quietly } - if (slave) - KIO::Scheduler::disconnectSlave(slave); } enum Operations { List = 0, ListRecursive, Stat, Get, Put, Copy, Move, Delete, Mkdir, Mimetype }; @@ -105,7 +103,6 @@ private: int selectedOperation; int progressMode; int putBuffer; - KIO::Slave *slave; }; #endif // KIOSLAVETEST_H