memcached_fetch_result can return previously returned data

[m6w6]

• Reported as: lp:1339334
• Reported by: lp:~martin-b69y0hv8h
• Reported at: 2014-07-08T20:38:24Z

Imported from Launchpad using lp2gh.

---

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.

[m6w6]

• Comment by: lp:~martin-b69y0hv8h:
• Created at: 2014-07-08T20:54:43Z

---

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.