Skip to content
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

busy loop in IPP client code #2179

Closed
michaelrsweet opened this issue Jan 3, 2007 · 6 comments

Comments

Projects
None yet
1 participant
@michaelrsweet
Copy link
Collaborator

commented Jan 3, 2007

Version: 1.2.7
CUPS.org User: twaugh.redhat

There is a busy loop condition in cupsDoFileRequest() in cups/request.c, here:

  while ((state = ippRead(http, response)) != IPP_DATA)
    if (state == IPP_ERROR)
    {
     /*
      * Delete the response...
      */

      DEBUG_puts("IPP read error!");
      ippDelete(response);
      response = NULL;

      _cupsSetError(IPP_SERVICE_UNAVAILABLE, strerror(errno));

      break;
    }

If ippRead() returns IPP_IDLE there is no delay before retrying. I was unable to catch the specific reason this kept happening indefinitely, but it was while sending a job to a 1.1.x server.

Original bug report: rhbz#219330

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 3, 2007

CUPS.org User: mike

Hmm, ippRead() should never return IPP_IDLE, but it looks like a read timeout when reading the initial response header will cause ippReadIO() to return it (even through response->state will be IPP_HEADER).

Try the attached patch and let me know if you still experience any difficulties...

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 15, 2007

CUPS.org User: twaugh.redhat

Still getting a report about the original problem, even with this patch.

There's a backtrace too:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=219330#c7

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 24, 2007

CUPS.org User: twaugh.redhat

The loop is:

  while ((state = ippRead(http, response)) != IPP_DATA)
    if (state == IPP_ERROR)
    {
     /*
      * Delete the response...
      */

      DEBUG_puts("IPP read error!");
      ippDelete(response);
      response = NULL;

      _cupsSetError(IPP_SERVICE_UNAVAILABLE, strerror(errno));

      break;
    }

but in ippRead->ippReadIO, ipp->data==IPP_ATTRIBUTE so we get here:

case IPP_ATTRIBUTE :
    while ((*cb)(src, buffer, 1) > 0)
    {
      ...
    }
    break;

and we fall out of the loop returning IPP_ATTRIBUTE.

Should we do something like this?:

case IPP_ATTRIBUTE :
if ((_cb)(src, buffer, 1) < 1)
return (IPP_ERROR); // like all the others
do
{
...
} while ((_cb)(src, buffer, 1) == 1);
break;

or is there some special reason that the IPP_ATTRIBUTE case is allowed to have no data available there?

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 24, 2007

CUPS.org User: twaugh.redhat

Or perhaps this sort of patch is needed?:

--- cups-1.2.7/cups/ipp.c.busy-loop 2007-01-24 13:45:50.000000000 +0000
+++ cups-1.2.7/cups/ipp.c 2007-01-24 13:47:24.000000000 +0000
@@ -1098,8 +1098,11 @@
break;

 case IPP_ATTRIBUTE :
  •    while ((*cb)(src, buffer, 1) > 0)
    
  •    for (;;)
    {
    
  •     if ((*cb)(src, buffer, 1) < 1)
    
  •       return (IPP_ERROR);
    
    • DEBUG_printf(("ippReadIO: ipp->current=%p, ipp->prev=%p\n",
                    ipp->current, ipp->prev));
      
@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2007

CUPS.org User: mike

Yes, that second version looks like the correct fix, committed in r

@michaelrsweet

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2007

"str2179.patch":

Index: ipp.c

--- ipp.c (revision 6176)
+++ ipp.c (working copy)
@@ -1054,7 +1054,7 @@
if ((n = (*cb)(src, buffer, 8)) < 8)
{
DEBUG_printf(("ippReadIO: Unable to read header (%d bytes read)!\n", n));

  •   return (n == 0 ? IPP_IDLE : IPP_ERROR);
    
  •   return (IPP_ERROR);
    

    }

    /*

@michaelrsweet michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.