C - Closure template class behaves strangely when multihreading depending on closure types-CodePudding

I am trying to write my own c wrapper class for linux using pthreads. The class 'Thread' is supposed to get a generic lambda to run in a different thread and abstract away the required pthread calls for that.

This works fine if the lambdas don't capture anything, however as soon as they capture some shared variables the behaviour seems to become undefined depending on whether or not the template types of two threads are the same or not. If both Thread objects caputre the same types (int and int*) it seems to (probably accidentally) work correctly, however as soon as i pass (e.g. like in my example) an integer A 'aka. stackInt' and an int ptr B 'aka. heapInt' to Thread 1 and only the int ptr B to Thread 2 i get a segfault in Thread 2 while accessing int ptr B.

I know that it must have something to do with the fact that each thread gets its own copy of the stack segment however i cant wrap my head around how that interferes with colsures capturing variables by refernce and calling them. Shouldn't the int ptr B's value point to the same address in each copy of it on the stack? How does the adress get messed up? I really can't wrap my head around whats the exact issue here..

Can anyone help me out here? Thank you in advance.

Here is the full example code:

class 'Thread'

// thread.h
#pragma once

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <unistd.h>


// ******************************* //
//         THREAD CLASS            //
// ******************************* //

template <typename C>
class Thread
{
private:
    C &m_closure;
    pthread_t m_thread;

public:
    Thread<C>(C &&closure)
        : m_closure(closure),
          m_thread()
    {}

    void start()
    {
        pthread_create(&m_thread, NULL, &Thread::threadFunction, (void *)this);
    }

    void join()
    {
        pthread_join(m_thread, NULL);
    }

private:
    void callbackOnInstance()
    {
        m_closure();
    }

    static void * threadFunction(void *);
};

template <typename C>
void * Thread<C>::threadFunction(void *caller)
{
    Thread<C> *callerObject = (Thread<C> *)caller;
    callerObject->callbackOnInstance();
    return nullptr;
}

main() / testing

// main.cpp

// ******************************* //
//           TESTING               //
// ******************************* //

#define SLEEP_SEC(_sec) usleep((long)(1000 * 1000 * (_sec)))

#include "thread.h"

#include <iostream>
#include <string>

int main(int argc, char **argv)
{
    int stackInt = 0;
    int *heapInt = new int(0);

    // every second each thread increments them, 0.5 s apart from each other

    Thread thread1([&]()
    {
        while(true)
        {
            SLEEP_SEC(1);

            std::cout << "thread #1:" << std::endl;

            stackInt  = 1;
            std::cout << "stack int: " << stackInt << " [" << &stackInt << "]" << std::endl;

            *heapInt  = 1;
            std::cout << "heap int: " << *heapInt << " [" << heapInt << "]" << std::endl;
        }
    });
    thread1.start();

    Thread thread2([&]()
    {
        SLEEP_SEC(0.5);

        while(true)
        {
            SLEEP_SEC(1);

            std::cout << "thread #2:" << std::endl;

            // if stackInt doesn't get referenced ...
            //stackInt  = 1;
            //std::cout << "stack int: " << stackInt << " [" << &stackInt << "]" << std::endl;

            // ... i get a segfault here
            *heapInt  = 1;
            std::cout << "heap int: " << *heapInt << " [" << heapInt << "]" << std::endl;
        }
    });
    thread2.start();

    thread1.join();
    thread2.join();
}

CodePudding user response：

You've got undefined behaviour, since the lambda objects are actually destroyed immediately after the constructor of Thread completes. To see this, instead of a lambda you could pass an object that prints a message in the destructor:

struct ThreadFunctor
{
    int& stackInt;
    int* heapInt;

    ThreadFunctor(int& si, int* hi)
        : stackInt(si),
        heapInt(hi)
    {
        std::cout << "ThreadFunctor created: " << this << '\n';
    }
    
    ~ThreadFunctor()
    {
        std::cout << "ThreadFunctor destroyed: " << this << '\n';
    }

    void operator()() const
    {
        using namespace std::chrono_literals;
        while (true)
        {
            std::this_thread::sleep_for(1s);

            std::cout << "thread #1:" << std::endl;

            stackInt  = 1;
            std::cout << "stack int: " << stackInt << " [" << &stackInt << "]" << std::endl;

            *heapInt  = 1;
            std::cout << "heap int: " << *heapInt << " [" << heapInt << "]" << std::endl;
        }
    }
};

Thread thread1(ThreadFunctor{stackInt, heapInt});
std::cout << "before start\n";
thread1.start();

The following output is guaranteed for every standard compliant C compiler (modulo addresses):

ThreadFunctor destroyed: 0000006E3ED6F6F0
before start
...

Furthermore the join operation only completes after the operation on the background thread has been completed, so because of the infinite loops your program won't terminate. You need some way of notifying the background threads to actually return instead of continuing forever.

Note that the standard library already contains the exact logic you're trying to implement here: std::thread or std::jthread for a implementation with builtin way of informing the background thread of a termination request.

int main()
{
    using namespace std::chrono_literals;

    int stackInt = 0;
    int* heapInt = new int(0);

    // every second each thread increments them, 0.5 s apart from each other

    std::jthread thread1{ [=](std::stop_token stopToken, int& stackInt)
    {
        using namespace std::chrono_literals;
        while (!stopToken.stop_requested())
        {
            std::this_thread::sleep_for(1s);

            std::cout << "thread #1:" << std::endl;

            stackInt  = 1;
            std::cout << "stack int: " << stackInt << " [" << &stackInt << "]" << std::endl;

            *heapInt  = 1;
            std::cout << "heap int: " << *heapInt << " [" << heapInt << "]" << std::endl;
        }
    }, std::ref(stackInt) }; // thread started immediately

    std::this_thread::sleep_for(10s);

    thread1.request_stop();
    thread1.join();
}