-
Notifications
You must be signed in to change notification settings - Fork 464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libcups attempted to connect to daemon endlessly if there's no daemon running #4870
Comments
Based on the Wine report this should be the same as #4867. As I'm using only the libraries and not the CUPS service this could explain why Wine hangs. |
Investigating, but I don't think we changed anything here that would cause infinite retries... :/ |
Testing on macOS shows we get the (expected) 30 second timeout and no retries. This might be Linux-specific... |
Yes, something is up with asynchronous connections on Linux... |
It seems the fix is incomplete. Compiled the sample programe with the latest cups source code. It still attempted to connect continually, and sent data into socket file descriptor -1
|
What happens if you run the "testcups" unit test program in the "cups" directory? In my Ubuntu VM the testcups program (which does a cupsGetDests call) exits immediately with an error. |
I ran the testcups program, and it still went into that endless loop on both ubuntu 16.04 and 16.10 without cups-daemon package installed. |
Hi, |
But now, when I changed only condition on line 302 (without application of your patch or any other changes) in http-addrlist.c to: if(result >= 0) testcups failed immediately and even 'make check' ran almost without fails (only one fail happened, but it was test, which was failing even before my change). Is it correct? |
In this case, "result" contains the number of file descriptors that connected. Since none have connected, returning an address with no connection is the wrong behavior. |
OK, I think I've found the "problem". The connection loop did not break out early if all of the addresses failed before the timeout. Thus, cupsGetDests (and other API functions) would take 30 seconds to timeout before continuing. [master e8916f4] Allow http*Connect to return early if all addresses fail (Issue #4870) |
Hi @michaelrsweet, It looks like this issue is still present in the released version 2.2.0. Here are bug reports from Arch Linux: For myself: after upgrading to libcups 2.2.0 the "gnome-settings-daemon" started to show 100% CPU usage. Attaching with gdb showed an infinite loop in |
I need to see a backtrace from gdb or ltrace output so we can see what is happening. |
Here is a looping backtrace, by attaching gdb to the
|
Same issue reported on downstream RedHat (Fedora) Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1366775 From this report:
I'm seeing the same backtrace, here are a few more details:
|
Reopening for now; with the changes in the released 2.2.0 I was unable to get this behavior in my Ubuntu and Fedora VMs, but I will re-test and try to duplicate. |
I think this bug is cause by this return: https://github.com/apple/cups/blob/master/cups/http-addrlist.c#L329 When cupsd is not running, nfds=1, fds[0].revents=POLLIN|POLLOUT|POLLERR|POLLHUP, the for loop will not assign value to neither *sock nor addrlist. But addrlist is not NULL because it has been assigned here https://github.com/apple/cups/blob/master/cups/http-addrlist.c#L239. The caller of httpAddrConnect2() will think the connection has been established. Add |
Xuzhen's patch worked for me. |
@michaelrsweet I found that If /var/run/cups/cups.sock already exists, connect() in httpAddrConnect2() tried to connect to it and failed with ECONNREFUSED, then everything worked as expected. But if /var/run/cups/cups.sock not exists, connect() tried to connect to localhost and failed with EINPROGRESS, then this bug appeared. Maybe this is the reason why this bug cannot be reproducedin your VMs |
@michaelrsweet I think the first issue, what Xu Zhen talks about, is solved by your previous patch ('Allow http*Connect to return early if all addresses fail'). To reproduce this, I needed to stop cups.service, cups.path and cups.socket. But to reproduce issue, which Xu Zhen's patch fixed, I needed to disable all these services in addition to stopping them. |
@zdohnal Yes, /var/run/cups/cups.sock was created by cups.socket unit at startup. Just stop the unit will not delete it. |
@xuzhen Your fix breaks printers/services that have more than one address. Which is basically always the case these days... |
OK, the poll/select that detects an error with any of the previous sockets will (incorrectly) return the next address in the list rather than continuing until all connections have failed. |
(and of course I messed up the commit message...) |
It worked for me |
Worked for me. |
Good, let's call this fixed then. 2.2.1 release will be released soon with this fix... |
(soon being in a week or so) |
* added patch for apple/cups#4870
CUPS 2.2rc1
Ubuntu 16.10
use the sample code from document:
the strace output:
The text was updated successfully, but these errors were encountered: