Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\))
Subject: Re: Issues/questions with apr_memcache_multigetp
From: Jim Jagielski <jim@jaguNET.com>
In-Reply-To: 
 <CA+7dWMGtFvoodXuntFeNFSAv=-+A0Y8u=-No6G49GDU+NT=uyQ@mail.gmail.com>
Date: Wed, 23 Sep 2015 15:48:30 -0400
Cc: dev@apr.apache.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <AF4A80B7-56EA-443C-A518-CCF0F15553D1@jaguNET.com>
References: 
 <CA+7dWMGtFvoodXuntFeNFSAv=-+A0Y8u=-No6G49GDU+NT=uyQ@mail.gmail.com>
To: Jeffrey Crowell <jcrowell@google.com>

Haven't looked at the actual bug but yet, in general, we appreciate
it when people send in patches that fix bugs or errors in our code
instead of simply forking off those changes.

Apache tries to work a little bit differently than the current buzz
about forking and working in isolation; we instead encourage people
to collaborate w/ the actual project itself, submitting bug reports
and patches, and working as a cohesive group, rather than scattered
islands of code :)

Thx!

> On Sep 23, 2015, at 3:23 PM, Jeffrey Crowell <jcrowell@google.com> =
wrote:
>=20
> Hi,
>=20
> I work on mod_pagespeed, where we use a forked version of =
apr_memcache.c =
(https://github.com/pagespeed/mod_pagespeed/blob/master/third_party/apruti=
l/apr_memcache2.c) to interact with memcached.
>=20
> Recently we've become aware of a bug where we end up hanging in =
apr_memcache2_multigetp, pinning cpu at 100%.
>=20
> We have some patches which were created in an attempt to fix some =
signed bugs in the original code that I think may be causing our issue.
>=20
> Namely here: =
https://github.com/pagespeed/mod_pagespeed/blob/master/third_party/aprutil=
/apr_memcache2.c#L1453  vs here =
https://gist.github.com/crowell/59bfa1bb9f0cda30c48a#file-multiget-c-L192
> The original code uses an atoi(), which has undefined behavior when =
called on non integer strings, leaving len to be 0.
>=20
> Second, the check here =
https://github.com/pagespeed/mod_pagespeed/blob/master/third_party/aprutil=
/apr_memcache2.c#L1457 vs =
https://gist.github.com/crowell/59bfa1bb9f0cda30c48a#file-multiget-c-L199
>=20
> In the original apr code, this check will never fail. len is an =
apr_size_t, and will never be < 0.
>=20
> The issue here now is that the check in the "server sent back a key =
that i didn't ask for" =
https://github.com/pagespeed/mod_pagespeed/blob/master/third_party/aprutil=
/apr_memcache2.c#L1507 (same in both forked and upstream), =
apr_pollset_remove, is never called, and the number of queries sent is =
never changed.
>=20
> Does it seem possible that this is causing the server to spin here, =
and would apr be interested in accepting a patch to fix the signedness =
issues?
>=20
> Thanks,
> Jeff