! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
[PATCH] Tree view: Expanding unmounted device tries to mount twice
Status:
RESOLVED: FIXED

Comments

Description Harald Judt 2012-10-25 22:02:44 CEST
I've finally found out what causes the mount errors in the tree view.

Steps to reproduce:
1) Start thunar, plug in USB stick.
2) Click on tree view pane whitespace so that the pane gets the focus.
3) Use down key to move cursor from Home folder to the device.
4) When the cursor is on the device, hit space to expand.

Expected results:
The device gets mounted, the tree item expands.

Actual results:
The device gets mounted, the tree item expands. The following error message appears: "DBus error org.gtk.Private.RemoteVolumeMonitor.Failed: An operation is already pending."

What the fuck? Now we actually got more than we wanted, and unnecessarily so. This is because the mount operation is happening _twice_.

So what the heck is going on here? In step 4), we expand. This causes the following to happen:
row_activated called.
test_expand_row: device not mounted, expanding.
mount_data_new called (open_after_mounting=0, open_in_new_window=0).
mount_data_new exit.
calling action_open.
treeview open called.
treeview open: device not mounted, calling treeview_mount.
treeview mount called.
mount_data_new called (open_after_mounting=1, open_in_new_window=0).
mount_data_new exit.
treeview mount: calling thunar_device_mount.
treeview mount exiting.
treeview open exiting.
row_activated exit.
treeview mount_finish called.
treeview mount_finish: error mounting device.
treeview mount_finish called.
treeview mount_finish: data->path != NULL, expanding row
treeview mount_finish exiting.
treeview mount_finish exiting.

Sorry about the inconsistent naming, but one can clearly see that the mount function is called twice; once by test_expand_row, which calls thunar_device_mount, and then by the open action which calls treeview mount which itself calls thunar_device_mount, because by the time the open action gets called, mounting of the device is still in progress.
Which means there are finally two mount operations going on on the same device at the same time. One of them can only result in an error.

Now there's the question what to do about it?

At first, I thought about adding some things to ensure awaiting the end of the mounting operation before opening the location. However, I got heavily confused by the many things one would need to take care of etc., and my head threatened to explode.

So I thought, when I can't add code to deal with this, why don't remove code so that I don't have to deal with it?

In fact, I don't see why that test-expand-row code is needed at all. It only checks if a device needs to be mounted, which will happen later anyway if needed (in action_open). The patch I attach here removes that redundant functionality completely. Signal test-expand-row is no longer dealt with.

I only use the tree view extremely rarely, so maybe I have overlooked something, but after removal of that code, everything works as before, and no error message occurs. It even got a bit simpler.
Comment 1 Harald Judt 2012-10-25 22:04:57 CEST
Created attachment 4692 
tree-view-do-not-mount-device-twice.patch
Comment 2 Harald Judt 2012-10-25 22:08:39 CEST
Aaahh. Ok, big fail. Now it doesn't open when I simply click on the expand widget. I've only tested this using the steps to reproduce in the original report.
Comment 3 Harald Judt 2012-10-25 22:51:10 CEST
Created attachment 4693 
tree-view-do-not-mount-device-twice.patch

I think I finally found a really simple solution. It doesn't delete as much code, but seems to work without any negative effects.
Comment 4 Nick Schermer editbugs 2012-10-28 19:28:32 CET
Looks harmless, pushed in 1eb2a9e.

Bug #9412

Reported by:
Harald Judt
Reported on: 2012-10-25
Last modified on: 2012-10-28

People

Assignee:
Jannis Pohlmann
CC List:
1 user

Version

Attachments

Additional information