Why am I getting a race condition?

Question

I'm trying to combine multiple CGAL meshes into one single geometry.

I have the following sequential code that works perfectly fine:

while (m_toCombine.size() > 1) {
    auto mesh1 = m_toCombine.front();
    m_toCombine.pop_front();
    auto mesh2 = m_toCombine.front();
    m_toCombine.pop_front();

    bool result = CGAL::Polygon_mesh_processing::corefine_and_compute_union(mesh1, mesh2, mesh2);

    m_toCombine.push_back(mesh2);
}

Where m_toCombine is a std::list<Triangle_mesh_exact>.

Triangle_mesh_exact is a type of CGAL mesh (triangulated polyhedron geometry). But I don't think it's really relevant to the problem.

Unfortunately, this process is way too slow for my intended application, so I decided to use the "divide to conquer" concept and combine meshes in a parallel fashion:

class Combiner
{
public:
    Combiner(const std::list<Triangle_mesh_exact>& toCombine) :
        m_toCombine(toCombine) {};
    ~Combiner() {};

    Triangle_mesh_exact combineMeshes();
    void combineMeshes2();

private:
    std::mutex m_listMutex, m_threadListMutex;
    std::mutex m_eventLock;

    std::list<MiniThread> m_threads;
    std::list<Triangle_mesh_exact> m_toCombine;
    std::condition_variable m_eventSignal;

    std::atomic<bool> m_done = false;

    //void poll(int threadListIndex);
};


Triangle_mesh_exact Combiner::combineMeshes()
{
    std::unique_lock<std::mutex> uniqueLock(m_eventLock, std::defer_lock);

    int runningCount = 0, finishedCount = 0;

    int toCombineCount = m_toCombine.size();

    bool stillRunning = false;
    bool stillCombining = true;

    while (stillCombining || stillRunning) {
        uniqueLock.lock();

        //std::lock_guard<std::mutex> lock(m_listMutex);
        m_listMutex.lock();
        Triangle_mesh_exact mesh1 = std::move(m_toCombine.front());
        m_toCombine.pop_front();
        toCombineCount--;
        Triangle_mesh_exact mesh2 = std::move(m_toCombine.front());
        m_toCombine.pop_front();
        toCombineCount--;
        m_listMutex.unlock();

        runningCount++;

        auto thread = new std::thread([&, this, mesh1, mesh2]() mutable {
            //m_listMutex.lock();
            CGAL::Polygon_mesh_processing::corefine_and_compute_union(mesh1, mesh2, mesh2);
            
            std::lock_guard<std::mutex> lock(m_listMutex);
            m_toCombine.push_back(mesh2);
            toCombineCount++;
            finishedCount++;
            m_eventSignal.notify_one();
            //m_listMutex.unlock();
        });
        thread->detach();
        
        while (toCombineCount < 2 && runningCount != finishedCount) {
            m_eventSignal.wait(uniqueLock);
        }

        stillRunning = runningCount != finishedCount;
        stillCombining = toCombineCount >= 2;

        uniqueLock.unlock();
    }
    
    return m_toCombine.front();
}

Unfortunately, despite being extra careful, I'm getting crashes of memory access violation or errors related to either mesh1 or mesh2 destructors.

Am I missing something?

Answer 1

Instead complicating things check capability of standard library:

std::reduce - cppreference.com

Triangle_mesh_exact combine(Triangle_mesh_exact& a, Triangle_mesh_exact& b)
{
    auto success = CGAL::Polygon_mesh_processing::corefine_and_compute_union(a, b, b);
    if (!success) throw my_combine_exception{};
    return b;
}

Triangle_mesh_exact combineAll()
{
    if (m_toCombine.size() == 1) return m_toCombine.front();
    if (m_toCombine.empty()) throw std::invalid_argument("");
    return std::reduce(std::execution::par,
                       m_toCombine.begin() + 1, m_toCombine.end(), 
                       m_toCombine.front(), combine);
}