0

I'm doing a wrapper class around win32 threads and pthreads, in a style similiar to the c++11 thread api. My problem is concerning updating the thread object instance when it's actual thread has exited. Below is a solution I have but I'm not sure if its safe.

/* Thread wrapper class */ class Thread { public: enum ThreadState { DETACHED = 0, RUNNING }; Thread(); Thread(void* (*start) (void*), void* arg); ~Thread(); Thread& operator=(Thread& other); void Join(); int32_t SetPriority(int32_t priority); void Detach(); ThreadHandle GetNativeHandle(); ThreadState GetThreadState(); private: ThreadHandle mHandle; ThreadState mState; void* (*mFunctionPointer) (void*); void* mArg; static void* Run(void* arg); ThreadHandle _CreateThread(void* (*start) (void*), void* arg); void _JoinThread(ThreadHandle& handle); };

The second constructor starts a thread. Here's the implementation:

 Thread::Thread(void* (*start) (void*), void* arg) { mFunctionPointer = start; mArg = arg; mState = Thread::RUNNING; mHandle = _CreateThread(&Run, (void*)this); if (!mHandle) mState = Thread::DETACHED; }

It creates a thread that runs the Run-method and passes along a pointer to this object instance. The reason is once the thread has executed the function it sets the state to DETACHED to signal it is done.

Here is the Run-method

 void* Thread::Run(void* arg) { Thread* thread = static_cast<Thread*>(arg); if (thread && thread->mFunctionPointer && thread->mArg && thread->mState == Thread::RUNNING) thread->mFunctionPointer(thread->mArg); if (thread && thread->mFunctionPointer && thread->mArg && thread->mState == Thread::RUNNING) thread->Detach(); return NULL; }

Also this is the Detach(), which is also called on thread destructor:

 void Thread::Detach() { mState = Thread::DETACHED; mHandle = NULL; mArg = NULL; mFunctionPointer = NULL; }

I feel like this is not safe at all. For example, if the Thread object was constructed on the stack and goes out of scope while its thread is running. The Thread object destructor NULLs its state and data members but the memory location might be overwritten no?

Is there any good way of solving this?

Thanks

Improve this question
2
  • 1
    FYI, Boost.Thread (boost.org/doc/libs/release/doc/html/thread.html) is a wrapper around win32 and pthreads. In the latest version of Boost (1.50), changes were made so that the API matches that of C++11's std::thead. Commented Jul 16, 2012 at 16:48
  • 1
    <rant> I quickly Googled a couple of C++11 std::thread pages. Both started off with examples using Thread->join(). It's like Deja vu all over again: create/terminate/join ad nauseum. Delphi, Java, C# and now C++11 - the same old rubbish. Have the members of standards committees ever actually written any non-trivial multithreaded software? Why are developers so caught up with join()? What textbook/webpage author is responsible? Lifetime-of-app threads and threadpools, fine, so what do we get: 'join()' </rant> Good luck anyway - you will need it:) Commented Jul 16, 2012 at 18:00

1 Answer 1

Reset to default
1

You are doomed, if the Thread instance goes out of scope without the thread being joined before. Make Run() a free or static function and copy all data, that the function need to run into dynamic allocated memory and let Run() free that memory.

There is no need for so much tests in Run(). As your c'tor arguments are already compatible with CreateThread(), just pass the function and the argument to CreateThread and you should be fine.

Kind regards

Torsten

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

If the object would perform Join() (if active thread) in the destructor, would it be considered safe then?
@KaiserJohaan sure, the thread, that invoked the d'or would block until the Run() function finished. But you usually don't want the d'tor to join the thread, because you might want to have more than one Thread object pointing to the same real thread of execution. If your Thread class would not be copyable, you could not store Thread objects in containers.
@TorstenRobitzki 'might want to have more than one Thread object pointing to the same real thread of execution' - I reserve the right to never, ever do such a thing :) Likewise building thread objects on stacks. Likewise using any container that is not a pointer type and so requiring any kind of 'real' copy ctor to load it up. 'myContainer<Thread>' bad, 'myContainer<*Thread> good :). I don't understand why anyone would try to build a thread object on another thread stack anyway - it is some attempt to carry RAII over to multithreading, because I see nothing but problems in doing so.
@MartinJames The Thread object is not the thread of execution, it's not a stack with a set of registered and maybe a CPU assigned to. Instead an object of type Thread is just a handle, a pointer or a reference to the Thread of execution. One reason to have a container of Threads might be to have a thread pool with a configurable size. Start n threads with the very same function, put all there handles (aka objects of class Thread) in a container and later join all thread from the container, if you want to stop the thread pool.
What Martin means keep only the thread pointer/handle and create a ThreadHelper/Wrapper that acts like a container - you give it handle and it just executes the function on it as dev like while for multithreading maintain list of threads in terms of thread-pointers not wrappers, and master thread can also use those wrappers..

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.