> On Oct. 19, 2015, 7:28 a.m., David Faure wrote: > > kio/kio/job.cpp, line 2671 > > <https://git.reviewboard.kde.org/r/125613/diff/3/?file=411228#file411228line2671> > > > > Same question here. As the comment says, "the result is still OK", > > which is why I didn't set an error code here. Please explain why this is > > needed. > > Alberto Jiménez Ruiz wrote: > The comment exactly says: (If we can't list a subdir, the result is still > ok). > > I interpret it like the result is not ok if the error is not about being > unable to list a directory. > > David Faure wrote: > Which other type of error would there be, when trying to list a > subdirectory? > > Whatever the error is, the result is that we were not able to list that > directory. > > (whether it's because of permission errors, or because it was deleted > during the listing by the user (race condition), or ... not sure what else > could happen, really) > > I think there's no reason for testing for a specific error code, and I > also think we don't want to setError, since "it's still OK".
That's fine, throwing just a warning signal should be fine. - Alberto ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125613/#review87048 ----------------------------------------------------------- On Oct. 19, 2015, 9:43 a.m., Alberto Jiménez Ruiz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125613/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2015, 9:43 a.m.) > > > Review request for kdelibs, Albert Astals Cid and David Faure. > > > Bugs: 333436 > http://bugs.kde.org/show_bug.cgi?id=333436 > > > Repository: kdelibs > > > Description > ------- > > Race condition and error notification loss in ListJob > > > Diffs > ----- > > kio/kio/job.cpp 91712e3 > kio/kio/jobclasses.h d771cfe > kio/tests/jobtest.h d3c552e > kio/tests/jobtest.cpp ee2677a > > Diff: https://git.reviewboard.kde.org/r/125613/diff/ > > > Testing > ------- > > Update 1, added unittest > Changed condition of listjob slotresult finishing (When executed as a > synchronous job, gets stuck in a eventloop) > Added setError to copyjob. > > > Tests done on Kubuntu 15.10: > > With this folder structure in ~: > src > ######c (Folder) > ##############b (Folder chmod'ed to 700 and owned by root) > #######################d (10MB file made of /dev/urandom data) > ##############a (10MB file made of /dev/urandom data) > > Then, (as a normal user execute: "kioclient ~/src ~/dst") > > Expected result: Notification that some files were not able to be copied > (Cannot access "b" folder) > Result with latest kdelibs: Silent copying where b folder is owned by user > but nothing inside. > ---When all kdebugdialog flags are set, a backtrace is also seen. > > subError notifications for listjob subjobs are lost, Copyjob uses this signal > to skip data. > > Also, this fix prevents a race condition. > > ListJob has another ListJob as a subjob. > > Chaild fails for whatever reason and calls error, which calls slotError, > which calls slotsFinished and emitResult. > The result of the child gets collected by parent. slotResult of parent gets > called, removes subjob and emitsresult because there are no subjobs left. > ---That's fine as long as the slave associated with the parent emits finished > before the child fails. If it doesn't, parent calls emitsResult twice. > > > Result after patch: kioclient shows a MessageBox telling the copy operation > could not be completed. > > > Thanks, > > Alberto Jiménez Ruiz > >
