tencentyun / cos-cpp-sdk-v5

MIT License
54 stars 46 forks source link

LRUCache<>::Get should not return a `const ValueType&`? #155

Closed mapleFU closed 3 months ago

mapleFU commented 5 months ago
  const ValueType& Get(const KeyType& key) {
    std::lock_guard<std::mutex> lock(m_mutex);
    auto it = m_entry_map.find(key);
    if (it == m_entry_map.end()) {
      throw std::range_error("No such key in cache");
    } else {
      m_entry_list.emplace_front(key, it->second->second);
      m_entry_list.erase(it->second);
      m_entry_map[key] = m_entry_list.begin();
      return m_entry_list.begin()->second;
    }
  }

Return a reference, when LRU Put is high and cache size isn't too many, might causing memory issue?

mapleFU commented 5 months ago

@Huberyxiao would you mind take a look?

Huberyxiao commented 5 months ago

In this scenario, the value is assigned to a conventional data structure, and no memory issues arise. However, this may lead to concurrency problems. The Get method returns a reference after releasing the lock, and subsequently, a copy operation is performed externally. During this process, if there is a concurrent Put method invocation, concurrency conflicts may occur. To address this issue, we consider altering the Get method to return a value type object instead of a reference

mapleFU commented 5 months ago

Yeah this is what I mean. Seems DNS cache would use it, it would raise problem when concurrently put/get the dns