Clean map periodically

Question

I have a A class managing a map.

class A {
 public:
  A() {}
  void addElem(uint8_t a, const B& b) {
    std::lock_guard<std::mutex> lock(_mutex);
    auto result = _map.emplace_hint(_map.end(), a, b);
    _deque.push_back(std::make_pair(result, time(nullptr)));
  }
  void cleanMap() {
    std::lock_guard<std::mutex> lock(_mutex);
    _map.erase(_deque.front().first);
    _deque.pop_front();
  }
 private:
  std::map<uint8_t, B> _map;
  std::deque<std::pair<std::map<uint8_t, B>::iterator, time_t>> _deque;
  std::mutex _mutex;
};

As I add a lot of elements in my map, I want to periodically clean it by removing elements that were first inserted.

if (difftime(time(nullptr), _deque.front().second > EXPIRY)) {
  cleanMap();
}

The following code is crashing at some point when I try to pop element from deque:

double free or corruption (fasttop): 0x00007fffdc000900 ***

Is the above code makes sense ? If yes, where could be the error ? And if not, how can I clean periodically a map ?

`periodically` - how and when and from where do you call the cleanMap function? Is it another thread? — KamilCuk, Mar 06 '19 at 09:56
I have a thread that called periodically the cleanMap function. — klaus, Mar 06 '19 at 09:59
Off topic but, as you've added a mutex to your code you should probably prefer the RAII style of (un)locking provided by e.g. [`std::lock_guard`](https://en.cppreference.com/w/cpp/thread/lock_guard) rather than explicit calls to lock/unlock. — G.M., Mar 06 '19 at 10:21

rafix07 · Accepted Answer · 2019-03-06T10:04:45.177

3

You have problems when you add the elements with the same key.

When emplace_hint is called with the key which exists, you push on deque duplicated iterators (emplace_hint returns the iterator to already existing element map::emplace_hint). When deque and map are cleared, you call map::erase but it accepts only valid and dereferenceable iterators. So when erase is called for the duplicated iterator (map::erase), the code crashes because this item was deleted in previous erase call.

edited Mar 06 '19 at 10:04

answered Mar 06 '19 at 09:55

rafix07

20,001
3
20
33

Thank you for the answer, you are probably right. So I need to check existence of the key and not pushing duplicated iterators ? – klaus Mar 06 '19 at 09:59
Yep, you need to find out how to store unique iterators, not duplicated. – rafix07 Mar 06 '19 at 10:00
@klaus Yes. Use `std::map::try_emplace`, which tells you whether the key already existed. Using `emplace_hint` only makes sense if you're sure that `a` increases from call to call. I mean, if you know that then just check for existence beforehand, but it's not clear from the example whether `emplace_hint` serves a purpose here. – Max Langhof Mar 06 '19 at 10:05
I added in elements in queue only when I was sure I did not add duplicated elements in my map and it worked. Do I need to protect addElem and cleanMap with mutex? – klaus Mar 06 '19 at 10:06
2

@klaus You added a comment under your question that you clean map from a thread, so you have to protect access to the map. – rafix07 Mar 06 '19 at 10:09

Clean map periodically

1 Answers1