Review Request: Fix for a couple of KIO put-slave-on-hold bugs
ahartmetz at gmail.com
Thu Apr 28 23:44:21 BST 2011
On Thursday 28 April 2011 16:05:51 Dawit Alemayehu wrote:
> This is an automatically generated e-mail. To reply, visit:
> (Updated April 28, 2011, 2:05 p.m.)
> Review request for kdelibs.
> Removed the m_ignoreOnHoldListChanged flag per discussion with David.
> The attached patch address a couple of bugs in the KIO put-slave-on-hold
> #1. If a user clicks a link on a page, e.g.
> ftp://ftp.kde.org/pub/kde/README_UPLOAD, then KRun will first issue a get
> (CMD_GET) to determine its content-type and put the ioslave on hold so
> that the application associated with the returned content-type can handle
> it. In case of our example that would be kate/kwrite. Unfortunately, the
> ioslave put on hold will not be reused because almost all applications
> will use KIO::file_copy (CMD_COPY) to make a local copy before opening it.
> Even then for ioslaves that do not optimize their copy command, the call
> to KIO::file_copy will properly reuse the ioslave on hold so long as it do
> not have an optimized copying method like the ftp ioslave.
> #2. If a user types a web address, e.g. www.kde.org, into KRunner to open
> it in Konqueror and repeats the same process with another address, then
> whether the ioslave put on hold the second time around will get reused or
> not depends how the application works. If the application allows multiple
> documents or tabs and opens the second url as in the already running
> instance, then the ioslave on hold will NOT be reused the second time
> #1. Simply force the KIO::file_copy call not to do the optimized copying if
> there is a slave on hold for the request. This will force it to use two
> separate jobs, KIO::get (CMD_GET) and KIO::put (CMD_PUT) ensuring the
> reuse of the ioslave that was put on hold.
> #2. Whenever a call to KIO::Scheduler::publishSlaveOnHold is made, send a
> dbus message to update all the other scheduler so that they can look for
> an ioslave-on-hold to service the next request.
> Diffs (updated)
> kio/kio/job.cpp e34f562
> kio/kio/scheduler.h 9be9db0
> kio/kio/scheduler.cpp 4cb33d1
> Diff: http://git.reviewboard.kde.org/r/101244/diff
The previous slave-on-hold patches trigger a -probably- previously existing
job accounting bug in the scheduler that was hidden before due to the mostly
non-working state of the slave-on-hold mechanism. AFAICS this bug was present
before my scheduler rewrite where I tried not to change anything I didn't
After enabling SCHEDULER_DEBUG I get the following debug output from
konqueror(5203)/kio (Scheduler) KIO::ProtoQueue::startAJob:
while some jobs stall and websites don't load. I can trigger this by running
e.g. konqueror http://paste.kde.org/39121/ (unrelated paste).
The bug is probably somewhere in KIO::SchedulerPrivate::putSlaveOnHold(),
publishSlaveOnHold(), or heldSlaveForJob(). The job's slave is never marked as
idle or not idle in any of these methods and it probably should be, somewhere.
Please take care of this before you continue. I know it's not really "your"
bug, but it's probably not really "mine" either.
Contact me if you'd prefer me to have a look. I am trying not to spend much
time on coding right now so I can spend more time on university stuff.
More information about the kde-core-devel