memcached_fetch_result can return previously returned data

Bug #1339334 reported by Martin C. Martin
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.

Revision history for this message
Martin C. Martin (martin-b69y0hv8h) wrote :

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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.