[gvfs/f20] Fix mtp crashes during unmount

Ondrej Holy oholy at fedoraproject.org
Fri Nov 29 13:09:42 UTC 2013


commit 01baf0671962db8a4a1408f8542dddc683a7341c
Author: Ondrej Holy <oholy at redhat.com>
Date:   Fri Nov 29 14:01:36 2013 +0100

    Fix mtp crashes during unmount

 ...-Fail-fast-if-in-the-middle-of-an-unmount.patch |  214 ++++++++++++++++++++
 gvfs.spec                                          |   10 +-
 2 files changed, 223 insertions(+), 1 deletions(-)
---
diff --git a/0001-MTP-Fail-fast-if-in-the-middle-of-an-unmount.patch b/0001-MTP-Fail-fast-if-in-the-middle-of-an-unmount.patch
new file mode 100644
index 0000000..57ba69a
--- /dev/null
+++ b/0001-MTP-Fail-fast-if-in-the-middle-of-an-unmount.patch
@@ -0,0 +1,214 @@
+From 807be442d3063247a9d05f16284475d2d9624d25 Mon Sep 17 00:00:00 2001
+From: Philip Langdale <philipl at overt.org>
+Date: Sun, 24 Nov 2013 09:12:37 -0800
+Subject: [PATCH] MTP: Fail fast if in the middle of an unmount
+
+I've seen a ton of bug reports where the backend crashes due to
+operations executing after an unmount begins. I think it's a
+sufficient solution to check the unmount flag that we already have
+and then immediately abort the operation.
+
+Generally, this is only seen with operations that are initiated
+implicitly like do_query_info or do_enumerate, but I've added the
+protection to all operations for consistency.
+---
+ daemon/gvfsbackendmtp.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
+ 1 file changed, 50 insertions(+)
+
+diff --git a/daemon/gvfsbackendmtp.c b/daemon/gvfsbackendmtp.c
+index 23e118b..728f1ad 100644
+--- a/daemon/gvfsbackendmtp.c
++++ b/daemon/gvfsbackendmtp.c
+@@ -324,6 +324,18 @@ remove_cache_entry_by_id (GVfsBackendMtp *backend,
+ }
+ 
+ 
++#define FAIL_DURING_UNMOUNT() \
++  if (g_atomic_int_get (&G_VFS_BACKEND_MTP(backend)->unmount_started)) { \
++    DEBUG ("(I) aborting due to unmount"); \
++    g_vfs_job_failed_literal (G_VFS_JOB (job), \
++                              G_IO_ERROR, G_IO_ERROR_CLOSED, \
++                              _("The connection is closed")); \
++    goto exit; \
++  }
++
++
++
++
+ /************************************************
+  * Initialization
+  ************************************************/
+@@ -1076,6 +1088,8 @@ do_enumerate (GVfsBackend *backend,
+ 
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   LIBMTP_mtpdevice_t *device;
+   device = op_backend->device;
+ 
+@@ -1162,6 +1176,8 @@ do_query_info (GVfsBackend *backend,
+   DEBUG ("(I) do_query_info (filename = %s) ", filename);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   gchar **elements = g_strsplit_set (filename, "/", -1);
+   unsigned int ne = g_strv_length (elements);
+ 
+@@ -1236,6 +1252,8 @@ do_query_fs_info (GVfsBackend *backend,
+   gchar **elements = g_strsplit_set (filename, "/", -1);
+   unsigned int ne = g_strv_length (elements);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   LIBMTP_mtpdevice_t *device;
+   device = G_VFS_BACKEND_MTP (backend)->device;
+ 
+@@ -1262,6 +1280,7 @@ do_query_fs_info (GVfsBackend *backend,
+ 
+   g_vfs_job_succeeded (G_VFS_JOB (job));
+ 
++ exit:
+   g_strfreev (elements);
+   g_mutex_unlock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
+@@ -1306,6 +1325,8 @@ do_make_directory (GVfsBackend *backend,
+   gchar **elements = g_strsplit_set (filename, "/", -1);
+   unsigned int ne = g_strv_length (elements);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   if (ne < 3) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+                               G_IO_ERROR, G_IO_ERROR_FAILED,
+@@ -1359,6 +1380,9 @@ do_pull (GVfsBackend *backend,
+ 
+   GFile *local_file = NULL;
+   GFileInfo *info = NULL;
++
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), source);
+   if (entry == NULL) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+@@ -1470,6 +1494,8 @@ do_push (GVfsBackend *backend,
+   gchar **elements = g_strsplit_set (destination, "/", -1);
+   unsigned int ne = g_strv_length (elements);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   if (ne < 3) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+                               G_IO_ERROR, G_IO_ERROR_NOT_REGULAR_FILE,
+@@ -1585,6 +1611,8 @@ do_delete (GVfsBackend *backend,
+   DEBUG ("(I) do_delete (filename = %s) ", filename);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), filename);
+   if (entry == NULL) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+@@ -1630,6 +1658,8 @@ do_set_display_name (GVfsBackend *backend,
+   DEBUG ("(I) do_set_display_name '%s' --> '%s' ", filename, display_name);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), filename);
+   if (entry == NULL) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+@@ -1698,6 +1728,8 @@ do_open_for_read (GVfsBackend *backend,
+   DEBUG ("(I) do_open_for_read (%s)", filename);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), filename);
+   if (entry == NULL) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+@@ -1749,6 +1781,8 @@ do_open_icon_for_read (GVfsBackend *backend,
+   DEBUG ("(I) do_open_icon_for_read (%s)", icon_id);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   guint id = strtol (icon_id, NULL, 10);
+ 
+   if (id > 0) {
+@@ -1824,6 +1858,8 @@ do_seek_on_read (GVfsBackend *backend,
+   DEBUG ("(I) do_seek_on_read (%u %lu %ld %u)", id, old_offset, offset, type);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   if (type == G_SEEK_END) {
+     offset = size + offset;
+   } else if (type == G_SEEK_CUR) {
+@@ -1861,6 +1897,8 @@ do_read (GVfsBackend *backend,
+   DEBUG ("(I) do_read (%u %lu %lu)", id, offset, bytes_requested);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   uint32_t actual;
+   if (handle->handle_type == HANDLE_FILE) {
+ #if HAVE_LIBMTP_1_1_6
+@@ -1943,6 +1981,8 @@ do_create (GVfsBackend *backend,
+   gchar **elements = g_strsplit_set (filename, "/", -1);
+   unsigned int ne = g_strv_length (elements);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   if (ne < 3) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+                               G_IO_ERROR, G_IO_ERROR_FAILED,
+@@ -2024,6 +2064,8 @@ do_append_to (GVfsBackend *backend,
+   DEBUG ("(I) do_append_to (%s)", filename);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), filename);
+   if (entry == NULL) {
+     g_vfs_job_failed_literal (G_VFS_JOB (job),
+@@ -2091,6 +2133,8 @@ do_replace (GVfsBackend *backend,
+   DEBUG ("(I) do_replace (%s)", filename);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   CacheEntry *entry = get_cache_entry (G_VFS_BACKEND_MTP (backend), filename);
+   if (entry == NULL) {
+     g_mutex_unlock (&G_VFS_BACKEND_MTP (backend)->mutex);
+@@ -2159,6 +2203,8 @@ do_write (GVfsBackend *backend,
+   DEBUG ("(I) do_write (%u %lu %lu)", id, offset, buffer_size);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   int ret = LIBMTP_SendPartialObject (G_VFS_BACKEND_MTP (backend)->device, id, offset,
+                                       (unsigned char *)buffer, buffer_size);
+   if (ret != 0) {
+@@ -2192,6 +2238,8 @@ do_seek_on_write (GVfsBackend *backend,
+   DEBUG ("(I) do_seek_on_write (%u %lu %ld %u)", id, old_offset, offset, type);
+   g_mutex_lock (&G_VFS_BACKEND_MTP (backend)->mutex);
+ 
++  FAIL_DURING_UNMOUNT();
++
+   if (type == G_SEEK_END) {
+     offset = size + offset;
+   } else if (type == G_SEEK_CUR) {
+@@ -2225,6 +2273,8 @@ do_close_write (GVfsBackend *backend,
+ 
+   RWHandle *handle = opaque_handle;
+ 
++  FAIL_DURING_UNMOUNT();
++
+   int ret = LIBMTP_EndEditObject (G_VFS_BACKEND_MTP (backend)->device, handle->id);
+   if (ret != 0) {
+     fail_job (G_VFS_JOB (job), G_VFS_BACKEND_MTP (backend)->device);
+-- 
+1.8.4.2
+
diff --git a/gvfs.spec b/gvfs.spec
index a2ffd3b..79f0284 100644
--- a/gvfs.spec
+++ b/gvfs.spec
@@ -3,7 +3,7 @@
 Summary: Backends for the gio framework in GLib
 Name: gvfs
 Version: 1.18.3
-Release: 1%{?dist}
+Release: 2%{?dist}
 License: GPLv3 and LGPLv2+
 Group: System Environment/Libraries
 URL: http://www.gtk.org
@@ -41,6 +41,10 @@ BuildRequires: libtool
 # http://bugzilla.gnome.org/show_bug.cgi?id=567235
 Patch0: gvfs-archive-integration.patch
 
+# Upstream patch to fix mtp crashes during unmount
+# https://bugzilla.gnome.org/show_bug.cgi?id=715119
+Patch1: 0001-MTP-Fail-fast-if-in-the-middle-of-an-unmount.patch
+
 Obsoletes: gnome-mount <= 0.8
 Obsoletes: gnome-mount-nautilus-properties <= 0.8
 Obsoletes: gvfs-obexftp < 1.17.91-2
@@ -162,6 +166,7 @@ file services.
 %prep
 %setup -q
 %patch0 -p1 -b .archive-integration
+%patch1 -p1 -b .fix-mtp-crashes
 
 # Needed for gvfs-0.2.1-archive-integration.patch
 libtoolize --force  || :
@@ -358,6 +363,9 @@ update-desktop-database >&/dev/null || :
 %{_datadir}/gvfs/remote-volume-monitors/goa.monitor
 
 %changelog
+* Fri Nov 29 2013 Ondrej Holy <oholy at redhat.com> - 1.18.3-2
+- Fix mtp crashes during unmount
+
 * Mon Nov 11 2013 Richard Hughes <rhughes at redhat.com> - 1.18.3-1
 - Update to 1.18.3
 


More information about the scm-commits mailing list