std::thread causing application to abort with error R6010

Question

I have a class named Task which internally holds a member std::thread. The general idea is to create a thread that stays alive as requests of processing come.

class Task
{
public:
    Task();
    ~Task();

    void start();
    // some funny stuff here
protected:
    Task(const Task& ref);
    void main_function();

    std::thread m_thread;
    // more funny stuff like queues, mutexes, etc
}

In the function start() I do:

void Task::start()
{
    m_thread = std::thread(std::bind(&Task::main_function, this));
}

The problem is that this line calls abort() with the runtime error R6010. I've read somewhere that this may be caused by the destructor of m_thread being called without a previous join but, as the thread hasn't started yet, I can't join it.

I'm running this with Visual Studio 2012

UPDATE:

So I tried a test example and could not replicate the error. Then I replaced my start function in the problematic function as sugested to this:

void Task::start()
{
    assert(!m_thread.joinable());
    m_thread = std::thread(&Task::main_function,this);
}

But I still get the error R6010. The call stack is:

msvcr110d.dll!_NMSG_WRITE(int rterrnum) Line 226    C
msvcr110d.dll!abort() Line 62   C
msvcr110d.dll!terminate() Line 97   C++
msvcp110d.dll!_Call_func(void * _Data) Line 63  C++

msvcr110d.dll!_callthreadstartex() Line 354 C
msvcr110d.dll!_threadstartex(void * ptd) Line 337   C

UPDATE2: Finally could replicate the problem. The code is as follows. Call foo() in the main function.

class Task 
{
public:
    Task() : m_exitFlag(false) 
    { 
        std::cout << "constructor called" << std::endl;
    }

    ~Task()
    {
        m_lock.lock();
        m_exitFlag = true;
        m_condlock.notify_all();
        m_lock.unlock();

        if (m_thread.joinable()) m_thread.join();
        std::cout << "destructor called" << std::endl;
     }

     void start()
     {
         std::cout << "Task start" << std::endl;
         assert(!m_thread.joinable());
         m_thread = std::thread(&Task::main_function, this);
     }
protected:
     void main_function()
     {
         std::cout << "thread started" << std::endl;
         while(1)
         {
             m_lock.lock();
             while(m_queue.empty() && !m_exitFlag)
                 m_condlock.wait(std::unique_lock<std::mutex>(m_lock));

             if (m_exitFlag)
             {
                 m_lock.unlock();
                 std::cout << "thread exiting" << std::endl;
                 return;
             }

             std::function<void()> f;
             if (!m_queue.empty()) f = m_queue.front();

             m_lock.unlock;
             if (f != nullptr) f();
         }
     }
     Task(const Task&ref) { }

     Task& operator=(const Task& ref) {
         return *this;
     }
};
void foo() {
    Task tk;
    tk.start();
}

I guess there's a race condition somewhere here, as it crashes some times and others don't. One thread is inside the critical region in ~Task() and the other as the stack in Update1.

Answer 1

Never directly lock a mutex. C++ offers lock_guard, unique_lock et al. for a reason.

In particular, this section is problematic:

m_lock.lock();
while(m_queue.empty() && !m_exitFlag)
    m_condlock.wait(std::unique_lock<std::mutex>(m_lock));

The newly constructed unique_lock will attempt to lock the already locked mutex m_lock. This will cause undefined behavior if the mutex is a std::mutex or a possible deadlock if the mutex is a std::recursive_mutex. Also note that this line is relying on a non-standard compiler extension as you bind the unnamed unique_lock to a non-const reference when calling wait.

So first thing you have to do is make the lock a named variable. Then either pass a std::adopt_lock to the lock's constructor or better yet never lock the mutex directly but always wrap it in an appropriate lock management class.

For example,

m_lock.lock();
m_exitFlag = true;
m_condlock.notify_all();
m_lock.unlock();

becomes

{
    std::lock_guard<std::mutex> lk(m_lock);
    m_exitFlag = true;
    m_condlock.notify_all();
} // mutex is unlocked automatically as the lock_guard goes out of scope

This has the additional benefit that you won't leak locks if an exception is thrown inside the critical section.

Answer 2

It seems you take a function from the front() of the queue and then run the function after unlocking without first removing it from the queue with a pop(). Is this your intention? In such a case the next random thread may also acquire the same function and run it concurrently. Are those functions thread-safe? Also, you check for queue.empty() but where is the queue being emptied of functions after they are executed?