Creating new thread causing exception

Question

I have a timer that will create a new thread and wait for the timer to expire before calling the notify function. It works correctly during the first execution, but when the timer is started a second time, an exception is thrown trying to create the new thread. The debug output shows that the previous thread has exited before attempting to create the new thread.

Timer.hpp:

 class TestTimer
{
private:
    std::atomic<bool> active;
    int timer_duration;
    std::thread thread;
    std::mutex mtx;
    std::condition_variable cv;
    void timer_func();
public:
    TestTimer() : active(false) {};
    ~TestTimer() {
        Stop();
    }
    TestTimer(const TestTimer&) = delete;               /* Remove the copy constructor */
    TestTimer(TestTimer&&) = delete;                    /* Remove the move constructor */
    TestTimer& operator=(const TestTimer&) & = delete;  /* Remove the copy assignment operator */
    TestTimer& operator=(TestTimer&&) & = delete;       /* Remove the move assignment operator */
    bool IsActive();
    void StartOnce(int TimerDurationInMS);
    void Stop();

    virtual void Notify() = 0;
};

Timer.cpp:

void TestTimer::timer_func()
{
    auto expire_time = std::chrono::steady_clock::now() + std::chrono::milliseconds(timer_duration);
    std::unique_lock<std::mutex> lock{ mtx };
    while (active.load())
    {
        if (cv.wait_until(lock, expire_time) == std::cv_status::timeout)
        {
            lock.unlock();
            Notify();
            Stop();
            lock.lock();
        }
    }
}

bool TestTimer::IsActive()
{
    return active.load();
}

void TestTimer::StartOnce(int TimerDurationInMS)
{
    if (!active.load())
    {
        if (thread.joinable())
        {
            thread.join();
        }
        timer_duration = TimerDurationInMS;
        active.store(true);
        thread = std::thread(&TestTimer::timer_func, this);
    }
    else
    {
        Stop();
        StartOnce(TimerDurationInMS);
    }
}

void TestTimer::Stop()
{
    if (active.load())
    {
        std::lock_guard<std::mutex> _{ mtx };
        active.store(false);
        cv.notify_one();
    }
}

The error is being thrown from my code block here: thread = std::thread(&TestTimer::timer_func, this); during the second execution.

Specifically, the error is being thrown from the move_thread function: _Thr = _Other._Thr;

thread& _Move_thread(thread& _Other)
        {   // move from _Other
        if (joinable())
            _XSTD terminate();
        _Thr = _Other._Thr;
        _Thr_set_null(_Other._Thr);
        return (*this);
        }

    _Thrd_t _Thr;
    };

And this is the exception: Unhandled exception at 0x76ED550B (ucrtbase.dll) in Sandbox.exe: Fatal program exit requested.

Stack trace:

thread::move_thread(std::thread &_Other)
thread::operator=(std::thread &&_Other)
TestTimer::StartOnce(int TimerDurationInMS)

Thank you Hans, coming from C I have a bit to learn on proper C++ class construction. I explicitly deleted the "special" member functions and edited my post to include the revised header. Though this didn't change the stack trace at all. The stack trace has been added to the post. — Bobby Tables, Sep 17 '18 at 14:07
Note that the above code has been edited to include the fix to the problem, notably failing to join the thread before attempting to create a new one. — Bobby Tables, Sep 17 '18 at 17:32

bobah · Answer 1 · 2018-09-17T14:39:32.673

2

If it's just a test

Make sure the thread handler is empty or joined when calling the destructor.
Make everything that can be accessed from multiple threads thread safe (specifically, reading the active flag). Simply making it an std::atomic_flag should do.

It does seem like you are killing a thread handle pointing to a live thread, but hard to say without seeing the whole application.

If not a test

...then generally, when need a single timer, recurreing or not, you can just go away with scheduling an alarm() signal into itself. You remain perfectly single threaded and don't even need to link with the pthread library. Example here.

And when expecting to need more timers and stay up for a bit it is worth to drop an instance of boost::asio::io_service (or asio::io_service if you need a boost-free header-only version) into your application which has mature production-ready timers support. Example here.

edited Sep 17 '18 at 14:39

answered Sep 17 '18 at 14:17

bobah

18,364
2
37
70

Yeah `alarm()` would be perfect, but unfortunately this app needs to be built with MSVC[apologies for not including the tag]. – Bobby Tables Sep 17 '18 at 16:13
1

@BobbyTables - then I'd totally go for [ASIO](https://think-async.com/Asio/AsioStandalone/) (or [Boost.ASIO](https://www.boost.org/doc/libs/1_66_0/doc/html/boost_asio.html)). – bobah Sep 17 '18 at 16:16
Thanks bobah, I'll try that next. – Bobby Tables Sep 17 '18 at 16:16
no probs... or the latest standard proposal reference implementation from the author of ASIO - https://github.com/chriskohlhoff/executors :-) – bobah Sep 17 '18 at 16:19
I'm going to have to stick to boost since all my libraries have to be "vetted", but the boost implementation should work, just waiting for it to unpack before I can try it out. – Bobby Tables Sep 17 '18 at 16:51

score 1 · Accepted Answer · answered Sep 17 '18 at 14:28

1

You create the TestTimer and run it the first time via TestTimer::StartOnce, where you create a thread (at the line, which later throws the exception). When the thread finishes, it sets active = false; in timer_func.
Then you call TestTimer::StartOnce a second time. As active == false, Stop() is not called on the current thread, and you proceed to creating a new thread in thread = std::thread(&TestTimer::timer_func, this);.

And then comes the big but:
You have not joined the first thread before creating the second one. And that's why it throws an exception.

answered Sep 17 '18 at 14:28

Mike van Dyke

2,724
3
16
31

The first thread is terminating. If I attempt to join the thread I also get an exception since the thread has already exited. – Bobby Tables Sep 17 '18 at 16:15
1

@BobbyTables you also have to join a thread which has already terminated. Which exception do you get? Also you should check if the thread is joinable, before using join. – Mike van Dyke Sep 17 '18 at 16:57
1

I finally got the correct symbols to load. The exception being thrown when I attempt to join the thread is: `if (get_id() == _STD this_thread::get_id())` `_Throw_Cpp_error(_RESOURCE_DEADLOCK_WOULD_OCCUR);` which means I'm trying to join it from inside the thread... oops. – Bobby Tables Sep 17 '18 at 17:19
Ok yeah, joining the thread before creating an new one fixed the issue as long as I wasn't trying to join itself ;) Editing my post to include the final version. – Bobby Tables Sep 17 '18 at 17:28

Creating new thread causing exception

2 Answers2