admin管理员组

文章数量:1318993

I needed to create a few threads in C++ and wrote this a few years ago. Im not sure why I created the static cast to spawn the task, but I remember it was the only way I could make the thread in a class. Can anyone please help me refresh my foggy brain?

#include <thread>
#include <cstdio>
#include <iostream>
#include "aclass.hpp"

void AClass::staticEntryPoint(void * c)
{
    return ((AClass *) c)->AClassThread();
}

void AClass::AClassThread(void)
{   
    while (!Global::g_terminate_all && !_terminate_me)
    {
        // thread task here
        std::this_thread::sleep_for(std::chrono::milliseconds(10));
    }
}

AClass::AClass(void)
{
    bool _terminate_me = false;

    std::thread txrxthreadObj(staticEntryPoint, this);
    txrxthreadObj.detach();
}

AClass::~AClass(void)
{
    _terminate_me = true;
}

I needed to create a few threads in C++ and wrote this a few years ago. Im not sure why I created the static cast to spawn the task, but I remember it was the only way I could make the thread in a class. Can anyone please help me refresh my foggy brain?

#include <thread>
#include <cstdio>
#include <iostream>
#include "aclass.hpp"

void AClass::staticEntryPoint(void * c)
{
    return ((AClass *) c)->AClassThread();
}

void AClass::AClassThread(void)
{   
    while (!Global::g_terminate_all && !_terminate_me)
    {
        // thread task here
        std::this_thread::sleep_for(std::chrono::milliseconds(10));
    }
}

AClass::AClass(void)
{
    bool _terminate_me = false;

    std::thread txrxthreadObj(staticEntryPoint, this);
    txrxthreadObj.detach();
}

AClass::~AClass(void)
{
    _terminate_me = true;
}
Share Improve this question asked Jan 19 at 22:42 Richard KlosinskiRichard Klosinski 1211 silver badge8 bronze badges 7
  • 3 The first line of your class's constructor is declaring and setting a local variable, which is probably not what you intended. – Jeremy Friesner Commented Jan 19 at 22:54
  • i think you were previously working with windows CreateThread or linux's pthread_create, which required you to pass a function that takes a void pointer, but now that you migrated to C++11 you don't need the function input to be a void* anymore as std::thread functions can take pointer that aren't to void .... or it can take any copyable object .. but you didn't update the function and still kept using the void* argument – Ahmed AEK Commented Jan 19 at 22:57
  • 1 also if _terminate_me is a class member then you needed to join the thread, not detach it, as after the destructor ends, reading _terminate_me from the other thread is UB. and will crash your program ... so make the thread a member of the class and join it in the destructor. – Ahmed AEK Commented Jan 19 at 23:00
  • 1 You do not need a static function and can use std::thread txrxthreadObj( &AClass::AClassThread, this );. – dalfaB Commented Jan 19 at 23:03
  • that's also disregarding the fact that concurrent access to bool from two threads is UB, the compiler can only check it once at the thread start then not check it anymore ... so _terminate_me needs to be a std::atomic<bool> ... and a member .... next to the std::thread member – Ahmed AEK Commented Jan 19 at 23:10
 |  Show 2 more comments

1 Answer 1

Reset to default 1

A std::thread object takes a unary function as argument that it invokes.

A class method needs the object as "implicit" argument.

You, for some reason, decide to pass that as void* to a static member function (which is not actually a method and has no instance to work on), and then need to cast it back to the correct type to be able to access members; that's a bit useless to have as a static member function, a freestanding function taking a pointer to your instance would have worked the same but less confusingly. The C-style cast and the fact you're doing this here at all is pretty suspicious – it says "C codebase adopted with minimal changes"; and that's not good here.

You wouldn't solve this problem like that, but simply do this with a capturing lambda, i.e., instead of your whole AClass:staticEntryPoint method, you'd use

std::thread txrxthreadObj([this](){ this->AClassThread();});

Other than that, this code is not inherently thread save. Multiple threads might be accessing _terminate_me concurrently.

Totally unsure why you have detach in there, that seems very questionable, especially since you're only stopping the thread in the destructor of the class. I can't really say whether and how you guarantee that resources used by the thread at that point are still available, but it does reek of being the wrong solution to an ownership problem.

本文标签: cStarting a thread from within a classStack Overflow