GNOME Bugzilla – Bug 710490
gvfsd-afp-browse consuming 100% cpu
Last modified: 2015-09-07 11:43:08 UTC
My cpu's fan is getting noisy and I just noticed gvfsd-afp-browse consuming 100% cpu. I can't tell exactly what it's doing so I attached gdb and break a few times to see what code path was hit. Here is the result : 0x00007f7b931272ba in g_private_get_impl (key=0x7f7b9339eb40 <private_thread_memory>) at /build/glib2.0-m8PF51/glib2.0-2.38.0/./glib/gthread-posix.c:994 994 /build/glib2.0-m8PF51/glib2.0-2.38.0/./glib/gthread-posix.c: No such file or directory. (gdb) bt
+ Trace 232641
I don't know the afpconnection code but it looks like read_all_cb in gvfsafpconnection.c does not handle reading 0 (i.e. EOF) from g_input_stream_read_finish which makes it spin endlessly.
Created attachment 257731 [details] [review] afpconnection: Fix 100% cpu consumption when connection is closed remotely Well spotted!
Review of attachment 257731 [details] [review]: If g_input_stream_read_finish() returns 0, it isn't really an error condition and error is not set so I'm not sure if calling g_simple_async_result_take_error() is a good idea in that case.
Created attachment 257742 [details] [review] afp: Don't retry read if connection closes
The attached patch sets the error condition manually if the connection closes and bytes are still expected.
*** Bug 710556 has been marked as a duplicate of this bug. ***
Review of attachment 257742 [details] [review]: I think we also have to change the the callers of read_all_finish() to not just do g_warning() and continue as nothing happened. For now I think the following should do: 1) Do "clean" shutdown on EOS, eg. shutdown using exit(). Print some debug output too. 2) Error out using g_error() on other errors. Doing proper shutdown on errors requires bigger changes which I may or may not get to some day :P
(In reply to comment #8) > Review of attachment 257742 [details] [review]: > > I think we also have to change the the callers of read_all_finish() to not just > do g_warning() and continue as nothing happened. > For now I think the following should do: > 1) Do "clean" shutdown on EOS, eg. shutdown using exit(). Print some debug > output too. > 2) Error out using g_error() on other errors. > As the afp backend expert, it would probably be better if you could work on a patch that does this? Thanks
Created attachment 282403 [details] [review] afp: Don't retry read if connection closes Don't retry the read if the remote host closes the connection. Instead, exit (semi-)gracefully with exit(0) and exit with an error if there was an actual error.
(In reply to comment #8) > I think we also have to change the the callers of read_all_finish() to not just > do g_warning() and continue as nothing happened. > For now I think the following should do: > 1) Do "clean" shutdown on EOS, eg. shutdown using exit(). Print some debug > output too. > 2) Error out using g_error() on other errors. > The latest update takes this approach and fixes the problem for me.
Looks good, thanks for doing all this work on the AFP backend.
(In reply to comment #12) > Looks good, thanks for doing all this work on the AFP backend. No problem, thanks for the review. If you're feeling charitable, can you take a look at bug 720806 and bug 733996. Thanks! Pushed to master as e9c320e49cace543450b961af32ea7ffeb8d512f.
Review of attachment 282403 [details] [review]: ::: daemon/gvfsafpconnection.c @@ +1060,3 @@ + { + g_message (_("Host closed connection")); + exit(0); Shouldn't be there g_vfs_backend_force_unmount() instead of exit()? @@ +1101,3 @@ + { + g_message (_("Host closed connection")); + exit(0); dtto
The problem is that after force_unmount, execution continues and the afp framework isn't geared to handle that. As Carl noted in comment 8, "Doing proper shutdown on errors requires bigger changes which I may or may not get to some day :P".
I've the same problem on ubuntu 14.04 64b (3.13.0-36-generic) connect to Synologie NAS with Nautilus = 100% CPU activity because of gvfs-afp and gvfs-browse. Sometime I've difficulty to unmount and have the message "unable to unmount because of the broken pipe error". I've to kill the process or reboot the computer...
Well that's because Ubuntu 14.04 doesn't have the version with the fix in it. File a bug with the distro and ask for them to update to 1.20.3 or add the patch manually.
(In reply to comment #17) > Well that's because Ubuntu 14.04 doesn't have the version with the fix in it. > File a bug with the distro and ask for them to update to 1.20.3 or add the > patch manually. Ok, thank you
Now there is about 20 abrt reports per week: https://retrace.fedoraproject.org/faf2/reports/424585/ https://bugzilla.redhat.com/show_bug.cgi?id=1173722 probably because: msg = 0x1315630 "FAIL!!! \"Error receiving data: Connection reset by peer\"\n" We should to exit "cleanly" also for G_IO_ERROR_CONNECTION_CLOSED.
Created attachment 298069 [details] [review] exit silently if connection closed
Review of attachment 298069 [details] [review]: You need to bump the glib version to 2.43.2 for G_IO_ERROR_CONNECTION_CLOSED. Otherwise looks good.
Created attachment 298276 [details] [review] afp: exit silently if connection closed Thanks for the review, there is patch also with GLib version dependency bump. (But maybe it would be enough to use G_IO_ERROR_BROKEN_PIPE instead without dependency bump...)
Review of attachment 298276 [details] [review]: Looks good.
Created attachment 298431 [details] [review] afp: exit silently if connection closed (gnome-3-14) It would be nice to fix it also for F20 to avoid abrt reports, but _CONNECTION_CLOSED error isn't supported there (already realized that _CONNECTION_CLOSED patch is from year 2011, but pushed recently). Seems G_IO_ERROR_FAILED is returned instead in older versions, so I think it would be best to change g_error to g_warning for gnome-3-14... do you agree?
Comment on attachment 298276 [details] [review] afp: exit silently if connection closed commit 8fdf64272ec47315cdd3e12d1aa628ad3bb837bc
Review of attachment 298431 [details] [review]: Looks good.
Comment on attachment 298431 [details] [review] afp: exit silently if connection closed (gnome-3-14) I forgot to delete check for G_IO_ERROR_CONNECTION_CLOSED, what is the reason why I was doing new patch, so pushed in gnome-3-14 without it as: commit 14369db2a8fc4f2a33e457a766b077ed6e2a3557
Thanks for reviews...
Sorry for reopening, but there are still some "crashes" caused by g_error... It is crashing because of the following error: "Errore nel ricevere i dati: Nessun instradamento per l'host" In the English it is something like (not sure about the source of the error to get right translation): "Error in receiving data: No route to host" It is from a downstream bug report: https://bugzilla.redhat.com/show_bug.cgi?id=1246811 The error doesn't have some special code probably. I don't know the afp code, but I suppose it would be best to change the g_error to g_warning at least and exit as I have done already for gnome-3-14, what do you think?
I guess the errror is probably either EHOSTUNREACH or ENETUNREACH caused by the network interface going down. I cherry-picked 14369db2a8fc4f2a33e457a766b077ed6e2a3557 from gnome-3-14 and altered the commit message. Thanks
Thanks! I've pushed it also for gnome-3-16: commit d663c85f0356d7800638c5eaa1a49e6c26bdfa33