Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit 84a9dea

Browse files
committed
fix apr_memcache multiget to prevent spinning
(issue #1048) in the case where if (strncmp(MS_VALUE, conn->buffer, MS_VALUE_LEN) == 0) { and else if (strncmp(MS_END, conn->buffer, MS_END_LEN) == 0) { both fail, it was possible for queries_sent to never decrement. This patch sets rv to APR_EGENERAL in this case, decrements the queries_sent, and closes the connection. According to the trace from betabrand this is where the hang is. Patch applied from apr dev mailing list (http://www.mail-archive.com/dev%40apr.apache.org/msg26265.html)
1 parent abc7860 commit 84a9dea

File tree

1 file changed

+53
-44
lines changed

1 file changed

+53
-44
lines changed

third_party/aprutil/apr_memcache2.c

Lines changed: 53 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,6 @@ apr_memcache2_multgetp(apr_memcache2_t *mc,
12861286
const apr_pollfd_t* activefds;
12871287
apr_pollfd_t* pollfds;
12881288

1289-
12901289
/* build all the queries */
12911290
value_hash_index = apr_hash_first(temp_pool, values);
12921291
while (value_hash_index) {
@@ -1441,8 +1440,9 @@ apr_memcache2_multgetp(apr_memcache2_t *mc,
14411440
char *length;
14421441
char *last;
14431442
char *data;
1443+
int length_ok;
14441444
apr_size_t len = 0;
1445-
int length_ok = 1;
1445+
apr_bucket *e = NULL;
14461446

14471447
key = apr_strtok(conn->buffer, " ", &last); /* just the VALUE, ignore */
14481448
key = apr_strtok(NULL, " ", &last);
@@ -1451,62 +1451,63 @@ apr_memcache2_multgetp(apr_memcache2_t *mc,
14511451

14521452
length = apr_strtok(NULL, " ", &last);
14531453
length_ok = (length == NULL) || parse_size(length, &len);
1454+
if (!length_ok) {
1455+
rv = APR_EINVAL;
1456+
}
1457+
else {
1458+
/* eat the trailing \r\n */
1459+
rv = apr_brigade_partition(conn->bb, len+2, &e);
1460+
}
1461+
if (rv != APR_SUCCESS) {
1462+
apr_pollset_remove (pollset, &activefds[i]);
1463+
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1464+
server_query, values, server_queries);
1465+
queries_sent--;
1466+
continue;
1467+
}
14541468
value = apr_hash_get(values, key, strlen(key));
14551469

14561470
if (value) {
1457-
if (length_ok) {
1458-
apr_bucket_brigade *bbb;
1459-
apr_bucket *e;
1460-
1461-
/* eat the trailing \r\n */
1462-
rv = apr_brigade_partition(conn->bb, len+2, &e);
1463-
1464-
if (rv != APR_SUCCESS) {
1465-
apr_pollset_remove (pollset, &activefds[i]);
1466-
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1467-
server_query, values, server_queries);
1468-
queries_sent--;
1469-
continue;
1470-
}
1471-
1472-
bbb = apr_brigade_split(conn->bb, e);
1473-
1474-
rv = apr_brigade_pflatten(conn->bb, &data, &len, data_pool);
1475-
1476-
if (rv != APR_SUCCESS) {
1477-
apr_pollset_remove (pollset, &activefds[i]);
1478-
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1479-
server_query, values, server_queries);
1480-
queries_sent--;
1481-
continue;
1482-
}
1483-
1484-
rv = apr_brigade_destroy(conn->bb);
1485-
if (rv != APR_SUCCESS) {
1486-
apr_pollset_remove (pollset, &activefds[i]);
1487-
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1488-
server_query, values, server_queries);
1489-
queries_sent--;
1490-
continue;
1491-
}
1492-
1493-
conn->bb = bbb;
1494-
1495-
value->len = len - 2;
1496-
data[value->len] = '\0';
1497-
value->data = data;
1471+
apr_bucket_brigade *bbb;
1472+
1473+
bbb = apr_brigade_split(conn->bb, e);
1474+
1475+
rv = apr_brigade_pflatten(conn->bb, &data, &len,
1476+
data_pool);
1477+
1478+
if (rv != APR_SUCCESS) {
1479+
apr_pollset_remove (pollset, &activefds[i]);
1480+
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1481+
server_query, values, server_queries);
1482+
queries_sent--;
1483+
continue;
1484+
}
1485+
1486+
rv = apr_brigade_destroy(conn->bb);
1487+
if (rv != APR_SUCCESS) {
1488+
apr_pollset_remove (pollset, &activefds[i]);
1489+
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1490+
server_query, values, server_queries);
1491+
queries_sent--;
1492+
continue;
14981493
}
14991494

1495+
conn->bb = bbb;
1496+
1497+
value->len = len - 2;
1498+
data[value->len] = '\0';
1499+
value->data = data;
1500+
15001501
value->status = rv;
15011502
value->flags = atoi(flags);
15021503

15031504
/* stay on the server */
15041505
i--;
1505-
15061506
}
15071507
else {
15081508
/* TODO: Server Sent back a key I didn't ask for or my
15091509
* hash is corrupt */
1510+
rv = APR_EGENERAL;
15101511
}
15111512
}
15121513
else if (strncmp(MS_END, conn->buffer, MS_END_LEN) == 0) {
@@ -1519,8 +1520,16 @@ apr_memcache2_multgetp(apr_memcache2_t *mc,
15191520
}
15201521
else {
15211522
/* unknown reply? */
1523+
fprintf(stderr,
1524+
"Caught potential spin in apr_memcache multiget!\n");
15221525
rv = APR_EGENERAL;
15231526
}
1527+
if (rv != APR_SUCCESS) {
1528+
apr_pollset_remove (pollset, &activefds[i]);
1529+
mget_conn_result(FALSE, FALSE, rv, mc, ms, conn,
1530+
server_query, values, server_queries);
1531+
queries_sent--;
1532+
}
15241533

15251534
} /* /for */
15261535
} /* /while */

0 commit comments

Comments
 (0)