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 777794 - Crashes in forced_unregister_mount_callback
Crashes in forced_unregister_mount_callback
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: mtp backend
git master
Other Linux
: Normal normal
: ---
Assigned To: Philip Langdale
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2017-01-26 14:59 UTC by Ondrej Holy
Modified: 2017-01-27 07:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backend: Hold reference during force unmount (1.32 KB, patch)
2017-01-26 15:01 UTC, Ondrej Holy
committed Details | Review
mtp: Disconnect uevent handler immediately (2.48 KB, patch)
2017-01-26 15:01 UTC, Ondrej Holy
committed Details | Review
gphoto2: Disconnect uevent handler immediately (2.17 KB, patch)
2017-01-26 15:01 UTC, Ondrej Holy
committed Details | Review
cdda: Disconnect uevent handler immediately (1.41 KB, patch)
2017-01-26 15:01 UTC, Ondrej Holy
committed Details | Review

Description Ondrej Holy 2017-01-26 14:59:42 UTC
MTP backend often crashes in forced_unregister_mount_callback. It happens also in gphoto2/cdda/afc backend, but not so often. I think that I find the cause, the problem is that g_vfs_backend_force_unmount is called multiple times...

See:
https://bugzilla.redhat.com/show_bug.cgi?id=1416636
https://retrace.fedoraproject.org/faf/problems/?component_names=gvfs&function_names=forced_unregister_mount_callback
Comment 1 Ondrej Holy 2017-01-26 15:01:15 UTC
Created attachment 344325 [details] [review]
backend: Hold reference during force unmount

g_vfs_backend_force_unmount may be called multiple times, or in
parallel with regular unmount operation. Hold backend reference during
the whole force unmount procedure in order to avoid unwanted segfaults.
Comment 2 Ondrej Holy 2017-01-26 15:01:27 UTC
Created attachment 344327 [details] [review]
mtp: Disconnect uevent handler immediately

Uevent handler with "remove" action may be called multiple times,
which causes that g_vfs_backend_force_unmount is called several
times, which may lead to segfault. Disconnect the uevent handler
immediately after g_vfs_backend_force_unmount call.
Comment 3 Ondrej Holy 2017-01-26 15:01:41 UTC
Created attachment 344328 [details] [review]
gphoto2: Disconnect uevent handler immediately
Comment 4 Ondrej Holy 2017-01-26 15:01:49 UTC
Created attachment 344329 [details] [review]
cdda: Disconnect uevent handler immediately
Comment 5 Philip Langdale 2017-01-27 05:15:50 UTC
Review of attachment 344327 [details] [review]:

Looks good to me.
Comment 6 Philip Langdale 2017-01-27 05:16:28 UTC
Review of attachment 344325 [details] [review]:

Ship it.
Comment 7 Philip Langdale 2017-01-27 05:16:41 UTC
Review of attachment 344328 [details] [review]:

Ship it.
Comment 8 Philip Langdale 2017-01-27 05:16:53 UTC
Review of attachment 344329 [details] [review]:

Ship it.
Comment 9 Ondrej Holy 2017-01-27 07:21:53 UTC
Attachment 344325 [details] pushed as 1ccf776 - backend: Hold reference during force unmount
Attachment 344327 [details] pushed as 32213bc - mtp: Disconnect uevent handler immediately
Attachment 344328 [details] pushed as d864620 - gphoto2: Disconnect uevent handler immediately
Attachment 344329 [details] pushed as cd4f923 - cdda: Disconnect uevent handler immediately
Comment 10 Ondrej Holy 2017-01-27 07:22:52 UTC
Thanks for the reviews!