GNOME Bugzilla – Bug 599664
The print dialog should not block while looking for an unplugged printer
Last modified: 2011-08-06 08:45:01 UTC
1. Have a printer attached to your computer and installed 2. Unplug the printer 3. Reboot your computer 4. Try to print something. The print dialog will block while trying to gather information for the (now unplugged) printer. Clicking on the other items in the dialog (e.g. on "Print to File") will have no immediate effect -> instead, the dialog should probe the printer in background and let the user click other entries in the meanwhile. Just happened on an up-to-date Fedora 12 (rawhide), with GTK+ 2.18.3.
I have just looked a bit into the issue and there seems to be a small bug in the non-blocking cups code. What happens is the following: * cups_dispatch_watch_prepare calls gtk_cups_request_read_write * gtk_cups_request_read_write calls _get_read_data * _get_read_data blocks inside httpRead2 (cups function) This is to be expected, because *_prepare is called to *prepar* the poll(), not to read data. At the same time gtk_cups_request_read_write needs to be called to open the connection to begin with. The attached patch fixes the issue by adding a new parameter connect_only to read_write, a more elegant solution might be to add a second function instead. So, after this, the gtk_cups_request_read_write in *_prepare will not block anymore (except for opening the tcp/ip connection to cups, I guess).
Created attachment 167373 [details] [review] patch that should fix the issue
OK, there is some more breakage there ... 1. GTK+ always poll()s for G_IO_OUT | G_IO_ERR. This is never changed, even when data neads to be read. 2. Now, if one fixes that, everything breaks. This happens because the call to ippRead by _post_read_response does not work correctly. Apprently this call needs to be called repeatedly even though there are no events on the FD. (Which works right now, because data could be *written* at all times, and therefore there is a tight loop reading the source)
Created attachment 167382 [details] [review] patch that fixes the issue OK. That one was kinda ugly to figure out. So, when posting the IPP data, we need to loop because of an internal cups buffer. When recieving GET data we need to keep track of the bytes recieved to know whether the file has ended, because of the HTTP connection keepalive (note that the internal buffer is irrelevant in this case, because GTK+ has quite a big buffer).
Created attachment 168498 [details] [review] small cleanup (without debug g_print, and a whitespace change)
Argh, the patch does not work (at least not on multi core machines apparently). Now it is hanging in _post_check, but I have not figured out why that happens.
Created attachment 168505 [details] [review] working patch OK, this patch is working fine finally, I think. I moved the loop to make sure the internal buffer from cups is empty to the *_read_write function.
Oh, shouldn't G_IO_HUP be in the event xor for GTK_CUPS_HTTP_WRITE instead of GTK_CUPS_HTTP_READ?
Hi Benjamin, I can not reproduce this. How do you reproduce it? Which version of CUPS do you have? (I'm testing it on Fedora 14). Regards Marek
I am using the patch on some production Debian testing machines which still use GTK+ 2.20.x; it is working great there and solves the problem described in the bug report. I am not able to reproduce it with GTK+ 2.22.x, which probably explains why you are not seeing it. However, the changes in the patch are valid and neccessary, and I will update the patch for GTK+ 2.22.x! Currently: * GTK+ currently polls for the wrong events when reading. * GTK+ does not take care of an internal Cups read buffer. not sure about that, but: * might hang until the connection is closed by cups when reading data.
Uhm, OK. The patch seems to apply cleanly to GTK+ 2.24 and master branches. Not sure why I did not seem to be able to apply them earlier.
Created attachment 192381 [details] [review] Patch to fix broken polling in GTK+ cups backend (GTK+ 3.x) This version of the patch is updated to GTK+ master (3.x)
Hi Benjamin, thank you for the patch. It looks good to me. However I have some comments on it. The first is name of the byte-count in _GtkCupsRequest. It should be "bytes_received" and not "bytes_recieved". The second thing is that we could join the check for bytes read in _get_read_data() with the previous one (the g_io_channel_write_chars() between them can handle the situation with "bytes == 0" well), so we can remove the first one and add the check (|| bytes == 0) to the second one. When we are in it, we can remove the first "request->poll_state = GTK_CUPS_HTTP_IDLE;" in gtk_cups_request_read_write() since it is set in the next condition. Maybe we could also make the call of "cups_dispatch_add_poll (source);" conditional with condition "if (!result)" because I think that we don't need to do the poll if we are ready to dispatch. It is ok for me to commit your patch with just the change from the first comment. Thank you Marek
Created attachment 193150 [details] [review] updated patch Thanks for the review! 1. Done 2. Yeah, the 0 byte write should be fine as we reached EOF anyways. Moved the check. 3. True. Removed the line. 4. I think it is more correct to leave it in. That way any old poll is removed and replaced by one with no events (0). I think that this should be good now. I'll push it in the next days if there is no objection. I guess I should also do a backport to GTK+ 2.x at the same time. Benjamin
I pushed the patch to both master and gtk-2-24.