GNOME Bugzilla – Bug 720058
Cancellation fails with push/pull when progress callbacks are used
Last modified: 2014-11-18 06:31:54 UTC
Cancelling works from the client point-of-view but the message doesn't get through to the daemon correctly.
Created attachment 263737 [details] [review] client: Use the correct serial with push/pull cancellation When using Push and Pull with progress callbacks, the progress callbacks increment the connection serial, so retrieve the serial immediately after the Push/Pull method call so that the correct serial is used for cancellation.
Review of attachment 263737 [details] [review]: Thanks for the patch. It looks good to me, however don't know the serials deeply... alex, could you look at please?
Review of attachment 263737 [details] [review]: ::: client/gdaemonfile.c @@ +2930,3 @@ copy_cb, &data); + serial = g_dbus_connection_get_last_serial (connection); Don't you need this for the _copy and _move calls also?
Created attachment 279941 [details] [review] client: Use the correct serial with file transfer cancellation When using Copy, Move, Push and Pull with progress callbacks, the progress callbacks increment the connection serial, so retrieve the serial immediately after the dbus method call so that the correct serial is used for cancellation.
(In reply to comment #3) > Review of attachment 263737 [details] [review]: > > ::: client/gdaemonfile.c > @@ +2930,3 @@ > copy_cb, > &data); > + serial = g_dbus_connection_get_last_serial (connection); > > Don't you need this for the _copy and _move calls also? Yes indeed. Fixed in latest patch. Thanks for the review.
Review of attachment 279941 [details] [review]: ::: client/gdaemonfile.c @@ +2978,3 @@ + serial); + else + _g_dbus_send_cancelled_sync (g_dbus_proxy_get_connection (G_DBUS_PROXY (proxy))); The only way this would be reached with serial != 0 is when g_dbus_interface_skeleton_export() fails, and at that point we didn't send an operation to cancel. So, maybe we should just check for serial != 0 instead of "if (proxy &&" and not send a dbus cancel if serial is 0 at this point.
Created attachment 286321 [details] [review] client: Use the correct serial with file transfer cancellation When using Copy, Move, Push and Pull with progress callbacks, the progress callbacks increment the connection serial, so retrieve the serial immediately after the dbus method call so that the correct serial is used for cancellation.
(In reply to comment #6) > Review of attachment 279941 [details] [review]: > > ::: client/gdaemonfile.c > @@ +2978,3 @@ > + serial); > + else > + _g_dbus_send_cancelled_sync (g_dbus_proxy_get_connection > (G_DBUS_PROXY (proxy))); > > The only way this would be reached with serial != 0 is when > g_dbus_interface_skeleton_export() fails, and at that point we didn't send an > operation to cancel. So, maybe we should just check for serial != 0 instead of > "if (proxy &&" and not send a dbus cancel if serial is 0 at this point. Thanks for the review, I have updated the patch accordingly.
Can this get committed? It's still very much an issue.
(In reply to comment #9) > Can this get committed? It's still very much an issue. The last patch I uploaded accidentally did not include all the changes, but I pushed a version with Alex's suggestions to master (a336ef412ace2f9fc169185c48bca657e28c646e) and stable (4b85af0d4a7ad69773839bd248284beb80e4bcac). Let me know if it's fixed.