GNOME Bugzilla – Bug 759061
sftp does not cancel jobs properly
Last modified: 2015-12-15 05:11:53 UTC
If you copy a large file to/from an sftp mount and then cancel it, the job is not finished and it subsequently blocks unmounting.
Created attachment 316803 [details] [review] sftp: Fail cancelled jobs Fail jobs which have been marked as cancelled, otherwise the job remains and blocks unmounting. This can be reproduced by cancelling while copying a large file to/from a mount and then trying to unmount it.
Review of attachment 316803 [details] [review]: Thanks for the patch, it fixes the mentioned problem... ::: daemon/gvfsbackendsftp.c @@ +5479,3 @@ g_clear_object (&handle->in); + if (g_vfs_job_is_finished (handle->job) || finish_cancelled_job (handle->job)) I just wonder why g_vfs_job_is_finished() is needed in those checks? If it is really needed, maybe I would move it into finish_cancelled_job() also and rename somehow...
Created attachment 317245 [details] [review] sftp: Fail cancelled jobs Fail jobs which have been marked as cancelled, otherwise the job remains and blocks unmounting. This can be reproduced by cancelling while copying a large file to/from a mount and then trying to unmount it.
(In reply to Ondrej Holy from comment #2) > Review of attachment 316803 [details] [review] [review]: > > Thanks for the patch, it fixes the mentioned problem... > > ::: daemon/gvfsbackendsftp.c > @@ +5479,3 @@ > g_clear_object (&handle->in); > > + if (g_vfs_job_is_finished (handle->job) || finish_cancelled_job > (handle->job)) > > I just wonder why g_vfs_job_is_finished() is needed in those checks? If it > is really needed, maybe I would move it into finish_cancelled_job() also and > rename somehow... g_vfs_job_is_finished() is used when there is more than concurrent outstanding action. For example, it might do a read from a local file and a write to the server (of previously read local data). If the write gets an error, it fails the job. When the read completes (push_read_cb) it needs to check whether the job has completed before it can continue. For this reason, I've updated the patch as you suggested.
Review of attachment 317245 [details] [review]: Looks good, thanks! It would be nice to push it also for stable...
Comment on attachment 317245 [details] [review] sftp: Fail cancelled jobs I've pushed the patch to master (commit 5e65aa2) and gnome-3-18 (commit 1085c64) myself in order to have it in the upcoming release...
(In reply to Ondrej Holy from comment #6) > Comment on attachment 317245 [details] [review] [review] > sftp: Fail cancelled jobs > > I've pushed the patch to master (commit 5e65aa2) and gnome-3-18 (commit > 1085c64) myself in order to have it in the upcoming release... Thanks!