After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 749314 - Cannot restore a just-trashed file
Cannot restore a just-trashed file
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.46.x
Other Linux
: High critical
: ---
Assigned To: gtkdev
gtkdev
: 755335 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-05-13 12:59 UTC by Iain Lane
Modified: 2015-10-14 17:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
reproducer (2.43 KB, text/x-csrc)
2015-05-13 12:59 UTC, Iain Lane
  Details
g_local_file_trash: write info file first (3.21 KB, patch)
2015-09-29 14:46 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Iain Lane 2015-05-13 12:59:01 UTC
Created attachment 303308 [details]
reproducer

GLib 2.45.1, not reproducible with 2.44

1. In nautilus, trash a local file (by hitting delete)
2. Visit the trash
3. Select the file and hit "Restore"

I get a message "Could not determine original location of “sintel-1024-surround.mp4”

The item cannot be restored from the Rubbish Bin" instead of having the item restored.

I think this is because gvfs doesn't know the original path (gvfs-info trash:///foo confirms this). If I restart gvfsd-trash then it this field is populated and the file can be restored.

Attached is a reproducer which fails with glib master but passes with 2.44.0. It prints whether the trashed item has "trash::orig-path" or not.
Comment 1 Sebastien Bacher 2015-09-29 07:40:41 UTC
the regression is in 2.46 now
Comment 2 Allison Karlitskaya (desrt) 2015-09-29 14:09:50 UTC
*** Bug 755335 has been marked as a duplicate of this bug. ***
Comment 3 Allison Karlitskaya (desrt) 2015-09-29 14:11:31 UTC
My analysis from the other bug (which on further research appears to be true):

This is related to the file monitor changes.

The biggest effect of those changes is that events are now reported immediately instead of waiting for 1 second.  It may be that the info file is not yet (fully) on disk at the point that the event is noticed.  The old code added a 1-second delay to everything, giving more time for the info file to make it to disk.

On the "trashing" side we look for a unique filename in the trash folder by creating an empty info file with that name and then moving the file to the trash and then writing out the contents of the info file.  It's pretty clear that gvfs-trash is now picking up the empty info file.

This is pretty insane -- I think the correct fix here is to write the full contents of the info file before moving the file into the trash.  That's a fix in g_local_file_trash()
Comment 4 Allison Karlitskaya (desrt) 2015-09-29 14:46:42 UTC
Created attachment 312363 [details] [review]
g_local_file_trash: write info file first

Recent changes to file monitors removed the delay before events were
reported.  Among other things, this caused the trash backend of gvfs to
notice trashed files sooner than before.

On noticing trashed files, the backend tries to read the info file to
discover (among other things) the original location of the file.

Unfortunately, g_local_file_trash() does a strange dance when trashing a
file.  It does a loop of open(O_EXCL) in order to file an empty filename
in the trash to write an info file to, trashes the file, and only then
writes the contents of the info file.  This means that at the time the
file is moved to the trash, the info file is an empty stub.

Change the order so that we write out the actual content of the info
file first.  If the actual trash files then we will unlink the info file
anyway.
Comment 5 Allison Karlitskaya (desrt) 2015-09-29 14:54:55 UTC
btw, trash spec has this to say:


"""

When trashing a file or directory, the implementation MUST create the corresponding file in $trash/info first.

"""

So the old code is definitely buggy.
Comment 6 Matthias Clasen 2015-09-29 15:03:37 UTC
Review of attachment 312363 [details] [review]:

Looks good to me, module a typo in the commit message "trash files" was meant to be "trash fails", I guess
Comment 7 David King 2015-09-29 15:33:58 UTC
Review of attachment 312363 [details] [review]:

I just tested this on Rawhide, and it fixes the bug for me.
Comment 8 Iain Lane 2015-09-29 15:37:45 UTC
Might it fix the related bug #749317 too? Could you comment/check please?
Comment 9 Allison Karlitskaya (desrt) 2015-10-14 17:08:29 UTC
Attachment 312363 [details] pushed as 8ece2de - g_local_file_trash: write info file first