memcached_fetch_result can return previously returned data

Bug #1339334 reported by Martin C. Martin on 2014-07-08
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

I think I've found the cause....

In memcached_instance_st::close_socket(), we reset read_buffer_length and read_ptr, but not read_data_length. So, read_data_length says we still have lots of data in the buffer, whereas read_data_length says it's empty.

If repack_input_buffer is called, we'll skip the initial "if" statement, then we'll try to read some more, but put the data at read_ptr + read_data_length, i.e. not at the start of the buffer, but further along.

I think I'm actually seeing this bug in practice. At least, I'm seeing old keys being returned by new requests in the presence of servers going away and coming back.

The read_data_length field really isn't needed. It's only ever read in repack_input_buffer(), and even then only when read_ptr == read_buffer, in which case it should be the same as read_buffer_length. Here's a patch which gets rid of it, without changing libmemcached's behavior except to fix the close_socket() issue.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers