Changes request: Thread notifications


#1

Hello, Julian.

 

What you think if I'd ask you to change the Thread class a bit so that it could notify a Listener on thread completion and also it could pass an exception on, thrown in a thread, to the notification callback. Just like C# BackgroundWorker class does. This C# class always call a callback on completion through caller's message queue on any thread completion (allowing you to call any other GUI functions safely),  including thread cancelation and thread exceptions. As I roughly see, it's possible to change source code this way:

void Thread::threadEntryPoint()
{
    const CurrentThreadHolder::Ptr currentThreadHolder (getCurrentThreadHolder());
    currentThreadHolder->value = this;
    JUCE_TRY
    {
        if (threadName.isNotEmpty())
            setCurrentThreadName (threadName);
        if (startSuspensionEvent.wait (10000))
        {
            jassert (getCurrentThreadId() == threadId);
            if (affinityMask != 0)
                setCurrentThreadAffinityMask (affinityMask);

            try
            {
                run();
            }
            catch (const ThreadException &e) { threadException = new ThreadException(e); }
        }
    }
    JUCE_CATCH_ALL_ASSERT
    currentThreadHolder->value.releaseCurrentThreadStorage();
    closeThreadHandle();

    triggerNotifyThreadExit();
}

 

If you are not going to implement the discribed functionality or if I'm not aware of something that is already implemented in JUCE and could be used to bring the functionality to my view then tell me how to acheive that without the proposed changes.


#2

Doesn't really feel like something that belongs in the thread class, when you could just have a one-line call at the end of your run() method that triggers a callback. What's the actual use-case that you have in mind when asking for this?


#3

It's not just one line. Callback must be called always on any completion. Like BackgroundWorker class says: RunWorkerCompleted event occurs when the background operation has completed, has been canceled, or has raised an exception.

 

I do not want to use timers or other workaround approaches just to figure out that a thread completed its job and a result is ready to use. I want to let my app to run smoothly (without any blocking) and react to a result only when it's ready. Just read the BackgroundWorker class documentation to see what it's capable of.

 

#4

I tried to develop a pattern for this situation and I've come up with this code:

 


template<typename Functor>
class FinallyGuard
{
public:
    FinallyGuard(Functor f) : _functor(std::move(f)), _active(true) {}
    FinallyGuard(FinallyGuard&& other) : _functor(std::move(other._functor)), active(other.active) { other.active = false; }
    ~FinallyGuard() { if (_active) _functor(); }

    void setActive(bool active = true) { _active = active; }
    bool isActive() const { return _active; }
    
    FinallyGuard& operator=(FinallyGuard&&) = delete;
    FinallyGuard& operator=(const FinallyGuard&) = delete;
    FinallyGuard() = delete;

private:
    Functor _functor;
    bool _active;
};

template<typename F> FinallyGuard<typename std::decay<F>::type> finally(F&& f) { return{ std::forward<F>(f) }; }


struct MessageWithTypeInfo : public Message
{
    MessageWithTypeInfo(const String &messageType) : _messageType(messageType) {}

    const String& getMessageType() const { return _messageType; }

    MessageWithTypeInfo() = delete;
    MessageWithTypeInfo(const MessageWithTypeInfo &) = delete;
    MessageWithTypeInfo& operator= (const MessageWithTypeInfo &) = delete;

private:
    String _messageType;
};


struct ThreadCompletionMessage : public MessageWithTypeInfo
{
    ThreadCompletionMessage(const String& messageType, const Thread& thread) : MessageWithTypeInfo(messageType), _thread(thread) {}

    const Thread& getThread() const { return _thread; }

    ThreadCompletionMessage() = delete;
    ThreadCompletionMessage(const MessageWithTypeInfo &) = delete;
    ThreadCompletionMessage& operator= (const MessageWithTypeInfo &) = delete;

private:
    const Thread &_thread;
};


class MyBackgroundWorker : public Thread
{
public:
    MyBackgroundWorker(MessageListener* listener = nullptr, const String& name = String::empty) : Thread(name), _listener(listener){}
    ~MyBackgroundWorker() { delete _exceptionPtr; }

    const std::exception *getException() const { return _exceptionPtr; }
    bool isCancelled() const { return _cancelled; }

protected:
    std::exception *_exceptionPtr{ nullptr };
    bool _cancelled{ false };
    MessageListener *_listener{ nullptr };

    void run() override
    {
        auto threadExitGuard{ finally([&] { triggerThreadExitNotify(); }) };

        try
        {
            std::cout << "Hello from thread!" << std::endl;

            if (threadShouldExit()) { _cancelled = true; return; }

            throw std::exception("exception");
        }
        catch (const std::exception& e) { _exceptionPtr = new std::exception(e); }
    }

    void triggerThreadExitNotify()
    {
        if (isCancelled())
            std::cout << "thread was cancelled..." << std::endl;
        else
        {
            if (getException() == nullptr)
                std::cout << "result is ready to use..." << std::endl;
            else
                std::cout << "thread exited with exception: '" << getException()->what() << "'" << std::endl;
        }

        if (_listener != nullptr)
            _listener->postMessage(new ThreadCompletionMessage("MyBackgroundWorker", *this));
    }
};


 

Julian, I want to know is this the right way to use JUCE with threads?

 


#5

Yes, that's a good way to do it! I wish everyone had a C++11 compiler so I could start using that kind of trick in the library!

I guess the only danger with it would be if the thread object were to get deleted before the message arrives, when it'd call a dangling pointer.

Your original request for a callback built into the thread class isn't a bad idea - it just feels like this kind of lambda-based callback is a more modern approach.


#6

Thank you, Julian, for good points!