Segmentation fault when emplacing to map-CodePudding

I have a map like this,

std::unordered_map<size_t, Connection> connections;

and I emplace elements into it like this,

void onConnectionEvent(size_t peer, std::string message, Local::TCP::Connection socket) {
    auto [element, inserted] = connections.try_emplace(peer);
    auto& connection = element->second;

    // Do Work
}

I was load testing the sever by making concurrent connections and at the 1000th connection the sever receives segfault and when debugging with GDB, I found that it happens because for some reason that it can't read variable inside the standard library's own emplacing code.

Existing answers on Stack Overflow say that this happens because program runs into infinite recursion or that stack size is not large enough, but I can't relate those causes to my problem.

I can't reproduce this all the time. It happens randomly.

Here is the standard library code where this happens:

/usr/include/c /11/bits/stl_function.h:

/// One of the @link comparison_functors comparison functors@endlink.
template<typename _Tp>
struct equal_to : public binary_function<_Tp, _Tp, bool>
{
  _GLIBCXX14_CONSTEXPR
  bool
  operator()(const _Tp& __x, const _Tp& __y) const

  // This is where this happens
  { return __x == __y; }
};

CodePudding user response：

You must use a lock that surrounds both manipulation and reading of the container.

STL containers themselves are not thread-safe by default, even though the way this is formulated is, in my opinion, rather confused. Manipulating the content of the same container instance itself - adding, removing or moving elements - is not thread-safe.

By default, the C memory model does not define what threads see when other threads manipulate data, unless there is forward progress. Manipulating a map potentially cause a lot of inter-related memory accesses. It is easy to create infinite loops or memory access violations if one thread only sees a partially modified map, or when that memory has been freed (when doing a resize, for example) by another thread.

You can enforce forward progress in many ways. In your case, you should protect all access to the list with a lock: anything that gets, adds, moves or removes and element from the list. If your container's iterator does not make a full copy of all elements, which is very likely, this includes iterators.

An example solution:

class ConnectionManager {
  std::unordered_map<size_t, Connection> connections;
  mutable std::mutex m;
  
public:
  void onConnectionEvent(size_t peer, std::string message, Local::TCP::Connection socket) {
    std::lock_guard guard(m);
    auto [element, inserted] = connections.try_emplace(peer);
    auto& connection = element->second;
    
    // Do Work
  }

  void handleConnection(std::function<void(std::unordered_map<size_t, Connection>) > handle) const {
    std::lock_guard guard(m);
    handle(connections);
  }
};

If you want to make sure multiple reads can happen any time, but modifying the map uses an exclusive lock:

class ConnectionManager {
  std::unordered_map<size_t, Connection> connections;
  mutable std::shared_mutex m;
  
public:
  void onConnectionEvent(size_t peer, std::string message, Local::TCP::Connection socket) {
    std::lock_guard guard(m);
    auto [element, inserted] = connections.try_emplace(peer);
    auto& connection = element->second;

    // Do Work
  }

  void handleConnection(std::function<void(std::unordered_map<size_t, Connection>) > handle) const {
    std::shared_lock guard(m);
    handle(connections);
  }
};