You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@wandisco.com> on 2012/10/29 22:31:38 UTC

EAGAIN handling in libsvn_ra_serf

In several places in ra_serf I see the pattern:

              /* Skip on to the next iteration of this loop. */
              if (APR_STATUS_IS_EAGAIN(status))
                {
                  return svn_ra_serf__wrap_err(status, NULL);
                }
              continue;

Is that test correct?  It causes EAGAIN to be returned and any other
error to continue the loop which seems to contradict the comment.  I
suppose EAGAIN could be handled at some higher level but simply
continuing on all other errors looks wrong.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download

Re: EAGAIN handling in libsvn_ra_serf

Posted by "C. Michael Pilato" <cm...@collab.net>.
On 10/29/2012 05:31 PM, Philip Martin wrote:
> In several places in ra_serf I see the pattern:
> 
>               /* Skip on to the next iteration of this loop. */
>               if (APR_STATUS_IS_EAGAIN(status))
>                 {
>                   return svn_ra_serf__wrap_err(status, NULL);
>                 }
>               continue;
> 
> Is that test correct?  It causes EAGAIN to be returned and any other
> error to continue the loop which seems to contradict the comment.  I
> suppose EAGAIN could be handled at some higher level but simply
> continuing on all other errors looks wrong.

Some crazy high percentage of ra_serf seems to be implemented as callbacks
for Serf itself, and I know Serf cares a good deal about EAGAIN, so perhaps
these call sites are just deferring the handling of that condition into the
appropriate hands?

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development


Re: EAGAIN handling in libsvn_ra_serf

Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@wandisco.com> writes:

> In several places in ra_serf I see the pattern:
>
>               /* Skip on to the next iteration of this loop. */
>               if (APR_STATUS_IS_EAGAIN(status))
>                 {
>                   return svn_ra_serf__wrap_err(status, NULL);
>                 }
>               continue;
>
> Is that test correct?  It causes EAGAIN to be returned and any other
> error to continue the loop which seems to contradict the comment.  I
> suppose EAGAIN could be handled at some higher level but simply
> continuing on all other errors looks wrong.

If I reverse the EAGAIN test then the regression tests hang so it
appears that returning EAGAIN and simply continuing for all other errors
does work.  I see serf itself has some EAGAIN code so perhaps that is
where it gets handled.  It's not clear to me how non-EAGAIN errors are
handled as they appear to simply get discarded.

-- 
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download