memcached_fetch_result can return previously returned data

[Martin C. Martin]

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.

[Martin C. Martin (martin-b69y0hv8h)]

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.