-> gvfs

Possibly a gvfs bug, Nautilus doesn't do anything special here.

If it helps, I can confirm this bug on openSUSE 12.3 with Gnome 3.6.
Only a kill via console or a restart of the X-server can get rid of this popup.

I can confirm it with Fedora 18, Gnome 3.6 and all updates installed. Neither "cancel" nor "unmount anyway" will do anything, the dialog just reappears and blocks the entire desktop. The only way to escape is to Ctrl-Alt-F2 and kill the gnome session.

Could someone please have a look into this bug? It's pretty annoying having to kill the entire session (and lose all unsaved work) because of this bug.

As a workaround you can simply mount smb/cifs shares via commandline. This has the positive side-effect of being way quicker compared to gvfs.

Created attachment 270501 [details] [review]
return G_IO_ERROR_CANCELLED when user cancelled it

This patch solve only a part of the problem. If the backend is busy, the dialog is shown. User can choose between "Force unmount" and "Cancel". If the dialog is cancelled, it is shown again after 2 seconds (if the backend is still busy). The dialog is really annoying, because it is system modal. There is no way to stop showing it, when we can't force unmount the backend. The error G_IO_ERROR_CANCELLED should be returned instead.

However it doesn't solve the problem, that it is impossible to force unmount the backend.

Maybe it will be better to return G_IO_ERROR_FAILED_HANDLED...

Created attachment 279608 [details] [review]
return G_IO_ERROR_FAILED_HANDLED when user cancelled it

Created attachment 279610 [details] [review]
return G_IO_ERROR_FAILED_HANDLED when user cancelled it

reupload, wrong patch attached firstly

Review of attachment 279610 [details] [review]:

There seem to be rather a lot of changes here. Could this not be done with *only* the addition of the following in unmount_cb?
  if (!should_unmount)
  {
    g_vfs_job_failed_literal (G_VFS_JOB (op_job),
                              G_IO_ERROR, G_IO_ERROR_FAILED_HANDLED,
                              "GMountOperation aborted");
    return;
  }

Before this patch, both "force unmount" and "cancel" segfaulted the smb backend for me. With this patch, only "force unmount" segfaults the backend, so it is some improvement at least. We really need to make unmounting, force unmounting, etc more robust :-)

::: daemon/gvfsjobunmount.c
@@ +196,3 @@
+                              G_IO_ERROR, G_IO_ERROR_FAILED_HANDLED,
+                              "GMountOperation aborted (user should never see this "
+                              "error since it is G_IO_ERROR_FAILED_HANDLED)");

I'd prefer a comment like this to be a comment in the code rather than compiled in to the executable, even if the user should never see it.

@@ -258,3 @@
     }
 
-  if (job_finish_immediately_if_possible (op_job))

With the removal of this, if a backend does not implement try_unmount or unmount, it is not possible to unmount it.

(In reply to comment #9)
> Review of attachment 279610 [details] [review]:
> 
> There seem to be rather a lot of changes here. Could this not be done with
> *only* the addition of the following in unmount_cb?
>   if (!should_unmount)
>   {
>     g_vfs_job_failed_literal (G_VFS_JOB (op_job),
>                               G_IO_ERROR, G_IO_ERROR_FAILED_HANDLED,
>                               "GMountOperation aborted");
>     return;
>   }

Right, it would be better. I wanted to remove job_finish_immediately_if_possible firstly, because we don't have backends without unmount (do we?)...

Created attachment 285394 [details] [review]
return G_IO_ERROR_FAILED_HANDLED when user cancelled it

So there is updated patch :-)

Created attachment 286215 [details] [review]
set callback to avoid endless loop during unmount

There is another problem when unmounting. It is not possible to response to unmount dialog after 2 sec timeout, because callback isn't set properly. The dialog is reopen every 2 sec until deamon is killed. It can be fixed by setting the callback on every timeout. However not sure this is right solution, couldn't this cause memory leaks?

Review of attachment 285394 [details] [review]:

This patch will be useless if patch 286219 from Bug 710986 is accepted.

(In reply to comment #12)
> Created an attachment (id=286215) [details] [review]
> set callback to avoid endless loop during unmount
> 
> There is another problem when unmounting. It is not possible to response to
> unmount dialog after 2 sec timeout, because callback isn't set properly. The
> dialog is reopen every 2 sec until deamon is killed. It can be fixed by setting
> the callback on every timeout. However not sure this is right solution,
> couldn't this cause memory leaks?

This should to fix also this downstream issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1139042

(In reply to comment #12)
> Created an attachment (id=286215) [details] [review]
> set callback to avoid endless loop during unmount
> 
> There is another problem when unmounting. It is not possible to response to
> unmount dialog after 2 sec timeout, because callback isn't set properly. The
> dialog is reopen every 2 sec until deamon is killed. It can be fixed by setting
> the callback on every timeout. However not sure this is right solution,
> couldn't this cause memory leaks?

From https://developer.gnome.org/gio/stable/GMountOperation.html#GMountOperation-show-processes :
Note that this signal may be emitted several times to update the list of blocking processes as processes close files. The application should only respond with g_mount_operation_reply() to the latest signal (setting “choice” to the choice the user made).

So the callback should be set properly for the latest signal...

Review of attachment 286215 [details] [review]:

While the original code doesn't seem to work at all, unfortunately I found that this sometimes causes on_show_processes_reply to be called twice causing the backend to segv. I need a little more time to investigate it fully...

(In reply to comment #16)
> Review of attachment 286215 [details] [review]:
> 
> While the original code doesn't seem to work at all, unfortunately I found that
> this sometimes causes on_show_processes_reply to be called twice causing the
> backend to segv. I need a little more time to investigate it fully...

Just to clarify, it seems to get called twice after only responding once from the client app.

Thanks, that you look on it. I will make also more testing, but I don't see such behavioral before...

Created attachment 287311 [details] [review]
daemon: Set callback to avoid endless loop during unmount

It is not possible to respond to unmount dialog after 2 sec timeout,
because the callback isn't set properly. The dialog reopens every 2
seconds until the daemon is killed.

Because a dbus method is called every 2 seconds, we may get multiple
replies (e.g. one normal reply and several error replies due to a closed
connection).  Handle this by ignoring all but the first reply.

Based on a patch by Ondrej Holy.

(In reply to comment #18)
> Thanks, that you look on it. I will make also more testing, but I don't see
> such behavioral before...

My test app was completing the unmount and then immediately exiting. This closed the dbus connection so all the outstanding replies would complete (probably with an error) and complete_unmount_with_op would get more than once, causing a segfault.  The attached patch (based on yours) fixes the problem.

Review of attachment 287311 [details] [review]:

Good point, thanks!

Pushed to master as ea347458a314ef1d99ed845b514f33b8af374ad7. Thanks for the review!

We should to backport this patch at least for gnome-3-14...

But it should be backported together with attachment 286802 [details] [review] to make sense, but not sure, whether we can do that since it is slightly changing behavioral of unmount operation...

Created attachment 290521 [details] [review]
gvfsjobunmount: don't unmount if dialog was cancelled

There is modified version of attachment 286802 [details] [review] targeted for stable releases. It changes only the case, when user cancelled the dialog. Other cases are untouched to be sure there won't be problems with changing api...

Could you please recheck it when it is targeted for stable releases (gnome-3-14 at least)?

Review of attachment 290521 [details] [review]:

Looks good to me!

Yes, getting attachment 290521 [details] [review] and attachment 287311 [details] [review] into some stable releases would be pretty good since this bug is really annoying.

Comment on attachment 290521 [details] [review]
gvfsjobunmount: don't unmount if dialog was cancelled

Thanks for review!

Both patches are committed into gnome-3-14 as:
commit 4cff08d1d9c458eb3ee46a4c4326626fffcaa7a8
commit 634b12a093779aac626757ab41d91ef07d7ce251

I pushed the patches also into gnome-3-12 finally as:
commit 71f4f71db67633a3984db480c814b7946d1faf7a
commit 6d51022f5af92fcbb05eace1a850f0ef72294cc8

and gnome-3-10 as:
commit 6344510ccebfbc73dd9ecf9106aa8234959156c7
commit a7d4b2e86c303642720138a9df188df35fb8d63f