On Thursday 28 April 2011 16:05:51 Dawit Alemayehu wrote: > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101244/ > ----------------------------------------------------------- > > (Updated April 28, 2011, 2:05 p.m.) > > > Review request for kdelibs. > > > Changes > ------- > > Removed the m_ignoreOnHoldListChanged flag per discussion with David. > > > Summary > ------- > > The attached patch address a couple of bugs in the KIO put-slave-on-hold > functionality: > > Problem: > ====== > #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 > around. > > Solution: > ====== > #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 > > > Testing > ------- > > > Thanks, > > Dawit
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 completely understand. After enabling SCHEDULER_DEBUG I get the following debug output from konqueror: konqueror(5203)/kio (Scheduler) KIO::ProtoQueue::startAJob: m_runningJobsCount: 3 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. Cheers, Andreas
