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 599664 - The print dialog should not block while looking for an unplugged printer
The print dialog should not block while looking for an unplugged printer
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Printing
2.18.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2009-10-26 14:59 UTC by Cosimo Cecchi
Modified: 2011-08-06 08:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch that should fix the issue (2.56 KB, patch)
2010-08-08 17:44 UTC, Benjamin Berg
none Details | Review
patch that fixes the issue (7.66 KB, patch)
2010-08-08 21:43 UTC, Benjamin Berg
none Details | Review
small cleanup (without debug g_print, and a whitespace change) (7.78 KB, patch)
2010-08-22 13:14 UTC, Benjamin Berg
none Details | Review
working patch (8.02 KB, patch)
2010-08-22 17:34 UTC, Benjamin Berg
none Details | Review
Patch to fix broken polling in GTK+ cups backend (GTK+ 3.x) (8.01 KB, patch)
2011-07-21 14:42 UTC, Benjamin Berg
none Details | Review
updated patch (7.79 KB, patch)
2011-08-03 10:01 UTC, Benjamin Berg
none Details | Review

Description Cosimo Cecchi 2009-10-26 14:59:05 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.
Comment 1 Benjamin Berg 2010-08-08 17:38:33 UTC
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).
Comment 2 Benjamin Berg 2010-08-08 17:44:01 UTC
Created attachment 167373 [details] [review]
patch that should fix the issue
Comment 3 Benjamin Berg 2010-08-08 20:08:22 UTC
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)
Comment 4 Benjamin Berg 2010-08-08 21:43:59 UTC
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).
Comment 5 Benjamin Berg 2010-08-22 13:14:00 UTC
Created attachment 168498 [details] [review]
small cleanup (without debug g_print, and a whitespace change)
Comment 6 Benjamin Berg 2010-08-22 16:20:14 UTC
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.
Comment 7 Benjamin Berg 2010-08-22 17:34:10 UTC
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.
Comment 8 Benjamin Berg 2010-08-22 18:24:04 UTC
Oh, shouldn't G_IO_HUP be in the event xor for GTK_CUPS_HTTP_WRITE instead of GTK_CUPS_HTTP_READ?
Comment 9 Marek Kašík 2010-11-22 13:40:51 UTC
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
Comment 10 Benjamin Berg 2010-11-23 15:41:37 UTC
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.
Comment 11 Benjamin Berg 2010-12-24 17:01:53 UTC
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.
Comment 12 Benjamin Berg 2011-07-21 14:42:23 UTC
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)
Comment 13 Marek Kašík 2011-08-01 15:30:10 UTC
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
Comment 14 Benjamin Berg 2011-08-03 10:01:33 UTC
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
Comment 15 Benjamin Berg 2011-08-06 08:45:01 UTC
I pushed the patch to both master and gtk-2-24.