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 733133 - Hang enumerating large directory
Hang enumerating large directory
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: afp backend
1.21.x
Other Linux
: Normal normal
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2014-07-13 20:56 UTC by Ross Lagerwall
Modified: 2014-07-17 16:50 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
afp: Fix race between writing and reading (2.74 KB, patch)
2014-07-13 21:56 UTC, Ross Lagerwall
committed Details | Review

Description Ross Lagerwall 2014-07-13 20:56:02 UTC
I can reproduce a hang about 20% of the time when enumerating a directory with 10000 files on one of my machines.

All the threads in the afp backend appear to be idle, but presumably there is some race condition.  I will debug it further.
Comment 1 Ross Lagerwall 2014-07-13 21:56:26 UTC
Created attachment 280605 [details] [review]
afp: Fix race between writing and reading

The following sequence of events is possible in GVfsAfpConnection:
send_request_unlocked
write_dsi_header_cb
... return to main loop ...
read_dsi_header_cb
write_command_cb

This happens if the server sends its response before the main loop gets
a chance to run since write_command_cb is executed asynchronously as an
idle function.  It causes the job to hang because the request is only
stored in the request hash table in write_command_cb and so the response
is ignored when being processed because it cannot find the corresponding
request.

To fix this, store the request in the hash table before the request is
written.
Comment 2 Ross Lagerwall 2014-07-13 21:57:37 UTC
Well that was fun to debug :-)
Comment 3 Ondrej Holy 2014-07-17 13:13:04 UTC
Review of attachment 280605 [details] [review]:

I haven't change to test it, but makes sense. Thanks for debugging!
Comment 4 Ross Lagerwall 2014-07-17 16:50:21 UTC
Pushed to master as 66582af10b29f5a94a8d437173ae396bbce84406. I thought I'd get it in before 3.13.4 for some extra testing. Thanks for the review.