search

hashicorp / consul

Consul is a distributed, highly available, and data center aware solution to connect and configure applications across dynamic, distributed infrastructure.
https://www.consul.io
Other
28.31k stars 4.42k forks source link

The Agent Cache should return copies of the RPC responses #5402

Open mkeeler opened 5 years ago

mkeeler commented 5 years ago

Overview of the Issue

When not using the agent cache, RPC responses returned can be considered "owned" by the caller. The data can be modified or filtered as necessary. However when the agent cache is in use the data cannot be modified as its being held onto by the cache itself and other code could be using it. In the best case that would result in incorrect data but in the worst it would result in consul crashing.

Suggested Fix

I think it would be okay to have the cache copy the cached RPC responses before returning them. It would be a little overhead but seems worth it in order to provide similar semantics to doing the regular RPC request.

This other PR is one example of a bug that can crop up because of the issue: https://github.com/hashicorp/consul/pull/5398

pierresouchay commented 5 years ago

@mkeeler yes, that's exactly what I suggested to @Aestek, but he chose to deal with it outside of cache to ease review, but I think you are right, copy should be returned