Review Request 125613: Race condition and error notification loss in ListJob

[David Faure]

> 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.

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".


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125613/#review87048
-----------------------------------------------------------


On Oct. 17, 2015, 2:02 p.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. 17, 2015, 2:02 p.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/copyjob.cpp e6c3817 
>   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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20151019/5eaff3e5/attachment.htm>