Jonas Eschmann
Jonas Eschmann

Reputation: 457

Mutex unlock fails strangely

I am playing around with some sockets, thread and mutexes. My question concerns threads and mutexes:

int ConnectionHandler::addNewSocket(){

    this->connectionList_mutex.lock();
    std::cout << "test1" << std::endl;
    this->connectionList_mutex.unlock();

    return 0;
}

int ConnectionHandler::main(){
    while(true){
        this->connectionList_mutex.lock();
        std::cout << "test2" << std::endl;
        this->connectionList_mutex.unlock();
    }

}`

The main function is running in one thread, while the addNewSocket is called by another thread. The problem is, that when addNewSocket is called once (by the second thread), the next unlock by thread 1 (main) will fail with a strange "signal SIGABRT". I have worked two days on this now, but i did not manage to get it fixed, sadly. I hope you can help me.

Edit: ConnectionHandler is a class, that has connectionList_mutex as a member.

Edit: Sometimes i also get this error: "Assertion failed: (ec == 0), function unlock, file /SourceCache/libcxx/libcxx-65.1/src/mutex.cpp, line 44." but it occurs randomly.

Edit: This is the whole class (Reduced to a minimum, should be context independant to a certain degree, but crashes when i put it right after a client connected, and works if i put it right after the start:

class ConnectionHandler{
public:
    ConnectionHandler();
    int addNewSocket();
private:
    int main();
    static void start(void * pThis);

    std::mutex connectionList_mutex;
};

ConnectionHandler::ConnectionHandler(){
    std::thread t(&this->start, this);
    t.detach();
}
void ConnectionHandler::start(void * pThis){
    ConnectionHandler *handlerThis;
    handlerThis = (ConnectionHandler *)pThis;
    handlerThis->main();
}


int ConnectionHandler::addNewSocket(){

    this->connectionList_mutex.lock();
    std::cout << "test1" << std::endl;
    this->connectionList_mutex.unlock();

    return 0;
}

int ConnectionHandler::main(){
    while(true){
        this->connectionList_mutex.lock();
        std::cout << "test2" << std::endl;
        std::this_thread::sleep_for(std::chrono::milliseconds(100));
        this->connectionList_mutex.unlock();

    }

}

Upvotes: 8

Views: 6644

Answers (1)

Omnifarious
Omnifarious

Reputation: 56048

My guess is that your ConnectionHandler object is being destroyed somewhere. Also, you have defined ConnectionHandler::start in a silly way.

First, ConnectionHandler::start should be defined this way:

void ConnectionHandler::start(ConnectionHandler * pThis){
    pThis->main();
}

The C++11 ::std::thread class is perfectly capable of preserving the type of the function argument so there is no need to resort to void *.

Secondly, add in this code:

void ConnectionHandler::~ConnectionHandler(){
    const void * const meptr = this;
    this->connectionList_mutex.lock();
    ::std::cout << "ConnectionHandler being destroyed at " << meptr << ::std::endl;
    this->connectionList_mutex.unlock();
}

And change the constructor to read:

ConnectionHandler::ConnectionHandler(){
    const void * const meptr = this;
    ::std::cout << "ConnectionHandler being created at " << meptr << ::std::endl;
    std::thread t(&this->start, this);
    t.detach();
}

This will show you when the ConnectionHandler object is being destroyed. And my guess is that your code is destroying it while your detached thread is still running.

The meptr thing is because operator << has an overload for void * that prints out the pointer value. Printing out the pointer value for this will allow you to match up calls to the constructor and destructor if you're creating multiple ConnectionHandler objects.

Edit: Since it turned out I was correct, here is how I would recommend you write the play ConnectionHandler class:

#include <iostream>
#include <atomic>
#include <thread>
#include <chrono>
#include <mutex>

class ConnectionHandler {
 public:
   ConnectionHandler();
   ~ConnectionHandler();
   ConnectionHandler(const ConnectionHandler &) = delete;
   const ConnectionHandler &operator =(const ConnectionHandler &) = delete;

   int addNewSocket();

 private:
   int main();
   static void start(ConnectionHandler * pThis);

   ::std::mutex connectionList_mutex;
   volatile ::std::atomic_bool thread_shutdown;
   ::std::thread thread;
};

ConnectionHandler::ConnectionHandler()
     : thread_shutdown(false), thread(&this->start, this)
{
}

ConnectionHandler::~ConnectionHandler()
{
   thread_shutdown.store(true);
   thread.join();
}

void ConnectionHandler::start(ConnectionHandler * pThis){
   pThis->main();
}

int ConnectionHandler::addNewSocket(){
   ::std::lock_guard< ::std::mutex> lock(connectionList_mutex);
   ::std::cout << "test1" << ::std::endl;

   return 0;
}

int ConnectionHandler::main(){
   while(!thread_shutdown.load()){
      ::std::lock_guard< ::std::mutex> lock(connectionList_mutex);
      ::std::cout << "test2" << ::std::endl;
      ::std::this_thread::sleep_for(::std::chrono::milliseconds(100));

   }
   return 0;
}

Upvotes: 6

Related Questions