Skip to content

Conversation

@adsr
Copy link
Contributor

@adsr adsr commented Feb 7, 2022

Hello, we ran into a case where instance->cursor_active_ underflows here:

memcached_server_response_decrement(instance);

leading to a near-infinite loop here:

while (instance->response_count()) {
(void) memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, &ptr->result);
}

It seems to occur once every couple hours on a staging host under Apache 2.4 and PHP 7.3 against memcached 1.6.6. We weren't able to pinpoint the exact conditions to reproduce, nor the source of the bug. Maybe a missing memcached_*_response_increment or memory corruption?

This appears to be a regression from 1.0.18 which we were running previously.

The patch mitigates but doesn't fix the bug.

@m6w6 m6w6 self-assigned this Feb 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2022

Codecov Report

Merging #124 (c8c52cf) into v1.x (bcc1a6e) will decrease coverage by 0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            v1.x    #124   +/-   ##
=====================================
- Coverage     68%     68%   -0%     
=====================================
  Files        194     194           
  Lines      17479   17479           
=====================================
- Hits       11930   11915   -15     
- Misses      5549    5564   +15     
Impacted Files Coverage Δ
src/libmemcached/common.h 100% <ø> (ø)
src/libmemcached/stats.cc 72% <0%> (-4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcc1a6e...c8c52cf. Read the comment docs.

@m6w6 m6w6 merged commit 54e7182 into awesomized:v1.x Nov 9, 2022
m6w6 added a commit that referenced this pull request Nov 9, 2022
* Fix [gh #130](#130)
  with [gh #124](#124):
  Server response count can underflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants