GNOME Bugzilla – Bug 550647
synchronous pipe I/O when reading mount reply
Last modified: 2008-09-09 15:43:50 UTC
I use Nautilus with automount enabled. On startup, Nautilus tries to mount all mountable volumes. This is done with a g_volume_mount() command. GIO uses “mount /media/floppy0” to mount my builtin floppy, and (in gunixvolume.c): data->error_channel_source_id = g_io_add_watch (data->error_channel, G_IO_IN, eject_mount_read_error, data); g_child_watch_add (child_pid, eject_mount_cb, data); The error channel is read out in eject_mount_read_error(), which reads: g_io_channel_read_to_end (channel, &str, &str_len, NULL); g_string_append (data->error_string, str); The read_to_end() call is problematic, because for the above mount command, the output is: LANG=C mount /media/floppy0 (~30 seconds of waiting) mount: block device /dev/fd0 is write-protected, mounting read-only (~30 seconds of waiting) mount: I could not determine the filesystem type, and none was specified We try to read_to_end() at the first “mount:” output, and because there was no EOF, we have to wait for the second “mount:” output (synchronously!). gunixmount.c seems to suffer from the same issue. Sidenote: I think we should also set the IO channels' locale to g_get_charset(). It causes a serious UI freeze, so I'll mark it as blocker.
Does this happen when using the hal volume monitor from gvfs? (It shouldn't) And, just curious, why aren't you using the hal volume monitor? A modern Linux desktop should be using that.
> Does this happen when using the hal volume monitor from gvfs? (It shouldn't) No it doesn't.
(In reply to comment #2) > > Does this happen when using the hal volume monitor from gvfs? (It shouldn't) > > No it doesn't. Good, thanks for testing. While I obviously can't speak for other gio developers, patches for fixing this are (of course) welcome... but I'm personally not going to work on fixing this corner case since GUnixVolumeMonitor et. al. isn't really supposed to be used on modern UNIX-based operating systems (neither, I think, are things like floppy drives).
> (neither, I think, are things like floppy drives). My computer doesn't even have any floppy drive, and that might be the reason for the extremely prolongated mount command. GIO just assumes that a floppy is present, even though there isn't. However, I think we all agree that it's a bad idea to ship anything with glib that potentially hangs user applications if a daemon happens not to run. Besides, we shouldn't expose to many flaky implementations of our own API - otherwise the API users (i.e. third-party developers) will copy them.
(In reply to comment #4) > > (neither, I think, are things like floppy drives). > > My computer doesn't even have any floppy drive, and that might be the reason > for the extremely prolongated mount command. (Your computer probably still has the FDC (floppy drive controller) just no drive attached to it. That's why the kernel happily creates /dev/fd0 + friends. And I suspect some "helpful" and misguided hardware detection script creates an /etc/fstab entry (more on that below). Of course the kernel *ought* to be checking for actual hardware but kernel dudes also likes to punt such things to user space. What a mess. Anyway, enough ranting.) > GIO just assumes that a floppy is > present, even though there isn't. So that's because your system is misconfigured and got a bogus entry in /etc/fstab (that's how GUnixVolumeMonitor "detects" hardware). If you haven't put the entry there yourself, complain to your OS vendor, tell them it's a bug to use /etc/fstab for that kind of stuff in modern Linux. > However, I think we all agree that it's a bad idea to ship anything with glib > that potentially hangs user applications if a daemon happens not to run. > Besides, we shouldn't expose to many flaky implementations of our own API - > otherwise the API users (i.e. third-party developers) will copy them. However, I think we all agree that it's a bad idea to optimize for situations that includes misconfigured systems or crashes in e.g. gvfs. You should still feel free to send patches against GUnixVolumeMonitor et. al. David
Created attachment 117994 [details] [review] Proposed patch Thanks for the analysis. Are you fine with the attached patch? I usually don't do lots of low-level programming, so a competent review would not be amiss. And please forgive me the following: <rant> Moreover, I am a little shocked how chauvinistic you (most of the developers?) refuse to handle system misconfigurations in higher levels of the system stack. Aren't we trying to deliver a *robust* desktop system for John Doe? Gosh, just because you know how to configure a system properly, you can not assume that everybody out there with a misconfigured system is stupid and does not deserve help. You are precisely showing the elitist geek attitude here that scares away the majority of people from using our system. Professionalism includes both technical and social competence. <end of rant>
I don't see how reading only part of the output is really a correct way to handle this. All this is done async anyway, no ? I don't see why this should cause any UI freeze in nautilus. What am I missing ?
> I don't see how reading only part of the output is really a correct way to handle this. g_io_channel_read_to_end() waits for EOF, and if the pipe is blocking, we seem to wait until the pipe is closed by the spawned mount process (=> sync I/O). > All this is done async anyway, no ? Asynchronously yes, but in-thread. In contrast to GFiles, the GVolume's asynchronous operations are not carried out in a worker thread, but this is proxied to the volume iface implementation's mount_fn() in the main thread. That said, if you don't like the partial reads, an alternative solution would be to use a worker thread.
Ah, ok. I think I understand now. So, I think we do want to read the complete output, but we don't want to block in g_io_channel_read_to_end. Looks like I misunderstood your patch and it actually does that ?
Oh, and btw, can you extend your patch to also squash the use of g_io_channel_read_to_end in gunixmount.c at the same time ?
> So, I think we do want to read the complete output, but we don't want to block > in g_io_channel_read_to_end. (...) (Y)our patch (...) does that ? Yes. > Oh, and btw, can you extend your patch to also squash the use of > g_io_channel_read_to_end in gunixmount.c at the same time ? That's already included in the patch. Thanks for the quick response and review. May I commit the patch?
Yeah, go ahead please.
Committed: http://svn.gnome.org/viewvc/glib?view=revision&revision=7444 Closing as fixed. Thanks everybody.
Closing as fixed as per last commit.