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 794957 - deadlock in g_vfs_daemon_close_active_channels
deadlock in g_vfs_daemon_close_active_channels
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2018-04-03 22:00 UTC by Jonathan Matthew
Modified: 2018-04-13 08:28 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
daemon: Prevent deadlock and invalid read when closing channels (2.11 KB, patch)
2018-04-10 13:33 UTC, Ondrej Holy 		committed Details | Review

Description Jonathan Matthew 2018-04-03 22:00:09 UTC 
Since the fix for bug 787992, g_vfs_daemon_close_active_channels will deadlock if there are any active channels.  The function itself holds daemon->lock, and job_source_closed_callback will attempt to take it again.

I can reproduce this reliably by suspending my laptop (fedora 27, gvfs 1.34.2.1) with an active sftp mount, then resuming several hours later.  The ssh process(es) close and gvfsd-sftp then gets stuck here:

+ Trace 238532

  • #0 syscall
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S line 38
  • #1 g_mutex_lock_slowpath
    at gthread-posix.c line 1313
  • #2 g_mutex_lock
    at gthread-posix.c line 1337
  • #3 job_source_closed_callback
    at gvfsdaemon.c line 407
  • #7 <emit signal ??? on instance 0x555e36ff88a0 [GVfsReadChannel]>
    at gsignal.c line 3447
  • #8 g_vfs_job_source_closed
    at gvfsjobsource.c line 107
  • #9 g_vfs_channel_force_close
    at gvfschannel.c line 796
  • #10 g_vfs_daemon_close_active_channels
    at gvfsdaemon.c line 1136
  • #11 forced_unregister_mount_callback
    at gvfsbackend.c line 1053
  • #12 g_task_return_now
    at gtask.c line 1145
  • #13 g_task_return
    at gtask.c line 1203
  • #14 reply_cb
    at gdbusproxy.c line 2589
  • #15 g_task_return_now
    at gtask.c line 1145
  • #16 g_task_return
    at gtask.c line 1203
  • #17 g_dbus_connection_call_done
    at gdbusconnection.c line 5722
  • #18 g_task_return_now
    at gtask.c line 1145
  • #19 complete_in_idle_cb
    at gtask.c line 1159
  • #20 g_idle_dispatch
    at gmain.c line 5486
  • #21 g_main_dispatch
    at gmain.c line 3142
  • #22 g_main_context_dispatch
    at gmain.c line 3795
  • #23 g_main_context_iterate
    at gmain.c line 3868
  • #24 g_main_loop_run
    at gmain.c line 4064
  • #25 daemon_main
    at daemon-main.c line 398
  • #26 main
    at daemon-main-generic.c line 45 






Comment 1


Ondrej Holy





          2018-04-06 17:00:17 UTC
        



Thanks for the report! Deadlock instead of segfault was not really intentional :-( I will try to prepare proper fix instead of just blind reversion of those patches...






Comment 2


Ondrej Holy





          2018-04-10 12:53:05 UTC
        



It seems that the this is more complicated than it looks. This bug just points to another bug in this code, which was there before. Signals are synchronous and thus g_vfs_channel_force_close causes removal of the current list item from daemon->job_sources and consequently there is an invalid read on l->next...






Comment 3


Ondrej Holy





          2018-04-10 13:33:11 UTC
        



Created attachment 370731 [details] [review]
daemon: Prevent deadlock and invalid read when closing channels

Commit e147e48 added missing mutex guards for job_sources, which may
unfortunately lead to deadlock because g_vfs_channel_force_close
synchronously calls g_vfs_job_source_closed which is also guarded by
the same mutex.

The deadlock reveals another bug which was in that code. The code
iterates over job_sources list, but g_vfs_job_source_closed removes
current element of the list, which leads to invalid reads and
potentially to segfaults also.

This patch tries to fix the both mentioned issues.






Comment 4


Ondrej Holy





          2018-04-13 08:28:34 UTC
        



Attachment 370731 [details] pushed as d35bab6 - daemon: Prevent deadlock and invalid read when closing channels

Going to push into affected stable branches as well...