C++ background thread in class - instance scope

Question

I have this simple class:

struct Foo {        
    void Run() {
        this->bgLoader = std::thread([this]() mutable {
            //do something 

            this->onFinish_Thread();
        });
    }
    
    std::function<void()> onFinish_Thread;
    std::thread bgLoader;
};

That is called from C-API:

void CApiRunFoo(){
    Foo foo;
    foo.onFinish_Thread = []() {
        //do something at thread end
    };
    foo.Run();
}

I want to run CApiRunFoo, return from it but keep the thread running until it is finished.

Now, the problem is, that once CApiRunFoo end, foo goes out of scope even if background thread is still running. If I change foo to object via new, it will run, but it will cause memory leak.

I was thinking to create destructor with:

~Foo(){
    if (bgLoader.joinable()){
        bgLoader.join();
    }
}

but I am not sure if it can cause deadlock or not plus it probably wont cause CApiRunFoo to return until the thread finishes.

Is there any solution/design pattern to this problem?

It's difficult to comment without knowing the *desired* behaviour. Do you want to ensure the loading thread has finished before returning from `CApiRunFoo`? — G.M., Apr 27 '21 at 06:55
I have edited the question - I want to run `CApiRunFoo`, return from it but keep the thread running until it is finished. — Martin Perry, Apr 27 '21 at 07:06

Ted Lyngmo · Accepted Answer · 2021-04-27T10:47:10.483

You could return the Foo instance to the C program:

struct Foo {        
    ~Foo() {
        if (bgLoader.joinable()) {
            run = false;
            bgLoader.join();
        }
    }
    void Run() {
        run = true;
        this->bgLoader = std::thread([this]() mutable {
            while(run) {
                // do stuff
            }

            this->onFinish_Thread();
        });
    }
    std::atomic<bool> run;
    std::function<void()> onFinish_Thread;
    std::thread bgLoader;
};

The C interface:

extern "C" {

struct foo_t {
    void* instance;
};

foo_t CApiRunFoo() {
    Foo* ptr = new Foo;
    ptr->onFinish_Thread = []() {
        std::cout << "done\n";
    };
    ptr->Run();
    return foo_t{ptr};
}

void CApiDestroyFoo(foo_t x) {
    auto ptr = static_cast<Foo*>(x.instance);
    delete ptr;
}

}

And a C program:

int main() { 
    foo_t x = CApiRunFoo();

    CApiDestroyFoo(x);
}

Demo

As it seems you'd like the Foo objects to automatically self destruct when the thread finishes, you could run them detached and let them delete this; when done.

#include <atomic>
#include <condition_variable>
#include <cstdint>
#include <iostream>
#include <functional>
#include <mutex>
#include <thread>

// Counting detached threads and making sure they are all finished before
// exiting the destructor. Used as a `static` member of `Foo`.
struct InstanceCounter {
    ~InstanceCounter() {
        run = false;
        std::unique_lock lock(mtx);
        std::cout << "waiting for " << counter << std::endl;
        while(counter) cv.wait(lock);
        std::cout << "all done" << std::endl;
    }
    void operator++() {
        std::lock_guard lock(mtx);
        std::cout << "inc: " << ++counter << std::endl;
    }
    void operator--() {
        std::lock_guard lock(mtx);
        std::cout << "dec: " << --counter << std::endl;
        cv.notify_one();      // if the destructor is waiting
    }
    std::atomic<bool> run{true};
    std::mutex mtx;
    std::condition_variable cv;
    unsigned counter = 0;
};

struct Foo {      
    bool Run() {        
        try {
            ++ic; // increase number of threads in static counter
            bgLoader = std::thread([this]() mutable {
                while(ic.run) {
                    // do stuff
                }
                // if onFinish_Thread may throw - you may want to try-catch:
                onFinish_Thread();
                --ic; // decrease number of threads in static counter       
                delete this; // self destruct
            });
            bgLoader.detach();
            return true;  // thread started successfully
        }
        catch(const std::system_error& ex) {
            // may actually happen if the system runs out of resources
            --ic;
            std::cout << ex.what() << ": ";
            delete this;
            return false; // thread not started
        }
    }

    std::function<void()> onFinish_Thread;

private:
    ~Foo() { // private: Only allowed to self destruct
        std::cout << "deleting myself" << std::endl;
    }

    std::thread bgLoader;
    static InstanceCounter ic;
};

InstanceCounter Foo::ic{};

Now the C interface becomes more like what you had in the question.

#include <stdbool.h>

extern "C" {

bool CApiRunFoo() {
    Foo* ptr = new Foo;
    ptr->onFinish_Thread = []() {
        std::cout << "done" << std::endl;
    };
    return ptr->Run();
    // it looks like `ptr` is leaked here, but it self destructs later
}

}

Demo

The only disadvantage is that API user must call destroy. I was thinking, if there is an "auto-destroy" solution, something like `shared_ptr`. — Martin Perry, Apr 27 '21 at 08:01
@MartinPerry I agree about the disadvantage - but that's how C interfaces usually work. You could however keep track of all the instances in C++ (in a `std::unordered_set`) to automatically destroy the instances (if not done explicitly via the C interface). The C interface's `CApiDestroyFoo` would then just remove the instance from the set to destroy it - and any forgotten instances would be destroyed at program termination. — Ted Lyngmo, Apr 27 '21 at 08:10
I was thinking if there is a way to transfer object ownership to thread and thread would destroy the object after calling `onFinish_Thread`. This way, I dont need to destroy object manually. With this, I cannot have join in dtor and thread shoud probably be detached. — Martin Perry, Apr 27 '21 at 08:17
@MartinPerry Sure, that's possible. Is [this](https://godbolt.org/z/ear7Thh9M) more like what you wanted? if so I may add that as an option to the answer with some explanations. — Ted Lyngmo, Apr 27 '21 at 09:10
Yes, its better, tahnk you. Add it, please, to the answer for future generations :) — Martin Perry, Apr 27 '21 at 10:07
@MartinPerry :-) Ok, added that (slightly modified to take care of resource exhaustion when spawning threads). — Ted Lyngmo, Apr 27 '21 at 10:30

score 0 · Answer 2 · answered Apr 27 '21 at 07:46

Your program should call join and finish the new thread at some point in future (see also this question with answer). To do that, it should hold a reference (in a wide sense) to the thread object. In your current design, your foo is such a reference. So you are not allowed to lose it.

You should think about a place where it makes sense to do join in your code. This same place should hold your foo. If you do that, there is no problem, because foo contains also the onFinish_Thread object.

C++ background thread in class - instance scope

2 Answers2