I wrote up a simple C++ wrapper class to encapsulate the (very basic)
usage of the ptheads API to create detached threads. My implementation
uses a global proxy function to call the thread object's member run/
execute function. Also, in my implementation I use a thread function
object to wrap/define the thread's run/execute function. The user is
expected to derive from the Thread FO (function object) and pass an
instance of it to the Thread object.
However, a simple test application using this Thread class crashes on
Linux with the following errors:
[some output from test program snipped]
pure virtual method called
terminate called without an active exception
Aborted
There's something horrendously wrong in my thread class implementation
and/or the test program. Can you please take a look at the source code
below and point out the errors in my code (and erroneous assumptions).
The system this is being run on is a Debian Etch system running the
2.6.19.7 kernel and the compiler is the default gcc/g++ 4.1.2 for
Debian Etch.
Thanks so much!
(thread.h)
#ifndef _THREAD_H_
#define _THREAD_H_
#include <pthread.h>
// forward declaration
class Thread;
// This is the Thread Function FO (function object). Derive from this
// class and override the function call operator to implement your
// Thread's Thread Function.
class ThreadFunction
{
public:
ThreadFunction(void) {}
virtual ~ThreadFunction(void) {}
// The thread function is passed a pointer to the owning Thread
// object. Please see the class declaration below for how to use
// the Thread class.
virtual void operator()(Thread* pThread) = 0;
};
// A simple Thread class that wraps POSIX threads.
// The created thread is a detached thread.
class Thread
{
public:
Thread(void);
Thread(ThreadFunction* pThreadFunc);
~Thread(void);
// Create and start execution of the (detached) Thread, using the
specified
// Thread Function FO and passing the specified Thread Argument to
it.
// returns 0 on success, (-1) on error.
int Create(ThreadFunction* pThreadFunc, void* pThreadArg, unsigned
int* pThreadID);
// Create and start execution of the (detached) Thread, passing
the specified
// Thread Argument to it.
// returns 0 on success, (-1) on error.
int Create(void* pThreadArg, unsigned int* pThreadID);
// Terminate this thread
// Note that the calling thread DOES NOT return from this call
void EndThread(void);
void Reset(void);
unsigned int GetThreadID(void) const
{
return static_cast<unsigned int>(m_thread);
}
// Accessor for the Thread Argument
void* GetThreadArg(void) const
{
return m_pThreadArg;
}
// Accessor for the Thread Function FO
ThreadFunction* GetThreadFunction(void) const
{
return m_pThreadFunc;
}
private:
pthread_t m_thread;
pthread_attr_t m_threadAttr;
void* m_pThreadArg;
ThreadFunction* m_pThreadFunc;
bool m_bCreated;
};
#endif // _THREAD_H_
(----------------------------------------------------------------
thread.cpp---------------------------------------)
#include <assert.h>
#include <pthread.h>
#include <stdio.h>
#ifndef _THREAD_H_
#include "thread.h"
#endif // _THREAD_H_
extern "C" {
void* thread_proxy(void* param)
{
assert(param != NULL);
if (param != NULL)
{
Thread* pThread = static_cast<Thread *>(param);
assert(pThread != NULL);
if (pThread != NULL)
{
(*pThread->GetThreadFunction())( pThread );
}
}
return 0;
}
}
// Terminate the calling thread
void Thread::EndThread(void)
{
pthread_exit(NULL);
}
void Thread::Reset(void)
{
(void)pthread_attr_destroy(&m_threadAttr);
m_pThreadArg = NULL;
m_pThreadFunc = NULL;
m_bCreated = false;
}
Thread::Thread(void)
: m_pThreadArg(NULL), m_pThreadFunc(NULL), m_bCreated(false)
{
}
Thread::Thread(ThreadFunction* pThreadFunc)
: m_pThreadArg(NULL), m_pThreadFunc(pThreadFunc),
m_bCreated(false)
{
}
Thread::~Thread(void)
{
// XXXI: There may be implications to having
// pthread_attr_destroy() being called on an already
// destroyed attribute object when the Reset() method
// has already been called prior to object destruction.
if (true == m_bCreated)
{
Reset();
}
}
// Create and start execution of the Thread, and if successful
// return the newly-created thread's ID in the threadID arg.
// returns 0 on success, (-1) on error.
int Thread::Create(ThreadFunction* pThreadFunc, void* pThreadArg,
unsigned int* pThreadID)
{
assert(pThreadFunc != NULL);
if (pThreadFunc != NULL)
{
m_pThreadFunc = pThreadFunc;
return Create(pThreadArg, pThreadID);
}
return (-1);
}
// Create and start execution of the Thread, and if successful
// return the newly-created thread's ID in the threadID arg.
// returns 0 on success, (-1) on error.
int Thread::Create(void* pThreadArg, unsigned int* pThreadID)
{
assert(m_pThreadFunc != NULL);
if (NULL == m_pThreadFunc)
{
return (-1);
}
if (true == m_bCreated)
{
// the thread has already been created - so do nothing.
return 0;
}
m_pThreadArg = pThreadArg;
if (pthread_attr_init(&m_threadAttr) < 0)
{
perror("pthread_attr_init() failed");
return (-1);
}
if (pthread_attr_setdetachstate(&m_threadAttr,
PTHREAD_CREATE_DETACHED) < 0)
{
perror("Failed to set DETACHED attribute on thread");
return (-1);
}
if (pthread_create(&m_thread, &m_threadAttr, &thread_proxy,
this))
{
perror("pthread_create() failed");
return (-1);
}
if (pThreadID != NULL)
{
*pThreadID = GetThreadID();
}
m_bCreated = true;
return 0;
}
// EOF
(------------------------------------------------------------
testthreads.cpp----------------------------------------)
#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <time.h> // for clock()
#include <unistd.h>
#ifndef _THREAD_H_
#include "thread.h"
#endif // _THREAD_H_
#define NUM_THREADS 10
struct ThreadData
{
int count;
int id;
unsigned int threadID;
ThreadData() : count(0), id(0), threadID(0)
{}
};
Thread threadArray[NUM_THREADS];
ThreadData thDataArray[NUM_THREADS];
class MyThreadFunction : public ThreadFunction
{
public:
virtual void operator()(Thread* pThread)
{
assert(pThread != NULL);
ThreadData* pData = static_cast<ThreadData *>(pThread-
>GetThreadArg());
assert(pData != NULL);
for (int i = 0; i < pData->count; i++)
{
sleep((rand() % 10) + 1);
}
printf("%d: Thread %u is exiting now.\n", pData->id, pData-
>threadID);
pThread->EndThread();
printf("%d: Thread %u returned from EndThread()\n", pData->id,
pData->threadID);
}
};
int
main(void)
{
MyThreadFunction threadFunction;
srand(clock());
// initialize thread data
for (int i = 0; i < NUM_THREADS; i++)
{
thDataArray[i].count = (i + 1);
thDataArray[i].id = 100 + (i + 1);
thDataArray[i].threadID = 0;
}
for (int i = 0; i < NUM_THREADS; i++)
{
if (threadArray[i].Create(&threadFunction, &thDataArray[i],
&thDataArray[i].threadID) < 0)
{
printf("Failed to create thread %d (error = %d)\n",
thDataArray[i].id, errno);
}
else
{
printf("successfully created thread %d\n",
thDataArray[i].id);
assert(0 == threadArray[i].Create(&threadFunction,
&thDataArray[i], &thDataArray[i].threadID));
}
}
printf("Main thread exiting.\n");
pthread_exit(NULL);
}
// EOF
(-----------------------------------------------
Makefile------------------------------------------)
CFLAGS+=-D_REENTRANT
CFLAGS+=-I.
CXXFLAGS=$(CFLAGS)
all: testthreads
clean:
$(RM) testthreads *~ core
testthreads: testthreads.cpp thread.h thread.cpp
$(CXX) $(CXXFLAGS) testthreads.cpp thread.cpp -o testthreads $
(LDFLAGS)-lpthread
|
|
0
|
|
|
|
Reply
|
fia_wrc_fanatic (10)
|
6/21/2007 6:59:13 PM |
|
fia_wrc_fanatic wrote:
> I wrote up a simple C++ wrapper class to encapsulate the (very basic)
> usage of the ptheads API to create detached threads. My implementation
> uses a global proxy function to call the thread object's member run/
> execute function. Also, in my implementation I use a thread function
> object to wrap/define the thread's run/execute function. The user is
> expected to derive from the Thread FO (function object) and pass an
> instance of it to the Thread object.
>
> However, a simple test application using this Thread class crashes on
> Linux with the following errors:
>
> [some output from test program snipped]
> pure virtual method called
> terminate called without an active exception
> Aborted
I doubt that this is a threading issue, but I'm to lazy to read all your
code. I think it's a call to a pure virtual function from a base class
constructor or destructor. Maybe the call stack might help you. If you
don't write your own classes to learn, I would use boost threads.
regards Torsten
|
|
0
|
|
|
|
Reply
|
Torsten
|
6/22/2007 7:12:14 AM
|
|
On Jun 22, 3:12 am, Torsten Robitzki <MyFirstn...@Robitzki.de> wrote:
>
> I doubt that this is a threading issue, but I'm to lazy to read all your
> code. I think it's a call to a pure virtual function from a base class
> constructor or destructor. Maybe the call stack might help you. If you
> don't write your own classes to learn, I would use boost threads.
>
You are correct, it was not a threading issue. Rather the issue had to
do with my (amateurish) C++ skills!
The problem was that I had declared the instance of MyThreadFunction
in main() which was duly getting whacked once the main thread went
away after creating the workers. The worker threads' run function was
trying to access a now non-existant memory address. The solution was
to move the the instance of MyThreadFunction to the global level i.e.
the same scope as the Thread objects themselves.
Cheers,
|
|
0
|
|
|
|
Reply
|
fia_wrc_fanatic
|
6/22/2007 5:17:26 PM
|
|
fia_wrc_fanatic wrote:
> #ifndef _THREAD_H_
> #define _THREAD_H_
You are not allowed to us macros beginning with an underscore and a capital,
those are reserved.
> void Thread::EndThread(void)
> {
> pthread_exit(NULL);
> }
I'm not exactly sure, but I think that this might cause threads not to call
destructors of local vars, i.e. not to unwind the stack. Also, btw,
the 'void' in an argument list is redundant in C++.
> (void)pthread_attr_destroy(&m_threadAttr);
^^^^^^
Ignoring errors is a really bad idea. At the very least invoke abort() or
signal it using exceptions.
> CFLAGS+=-D_REENTRANT
[...]
> testthreads: testthreads.cpp thread.h thread.cpp
> $(CXX) $(CXXFLAGS) testthreads.cpp thread.cpp -o testthreads $
> (LDFLAGS)-lpthread
Assuming GCC, compile with -pthread instead of linking to libpthread and
defining _REENTRANT manually.
Other than that, I'd second the suggestion to look at Boost.Thread.
Uli
|
|
0
|
|
|
|
Reply
|
Ulrich
|
6/22/2007 5:49:18 PM
|
|
fia_wrc_fanatic wrote:
> The problem was that I had declared the instance of MyThreadFunction
> in main() which was duly getting whacked once the main thread went
> away after creating the workers. The worker threads' run function was
> trying to access a now non-existant memory address. The solution was
> to move the the instance of MyThreadFunction to the global level i.e.
> the same scope as the Thread objects themselves.
No, this is not a solution but a workaround. There are two very fundamental
problems with your code, both rather C++-related than threading-related:
1. Using pointers with unclear ownership.
2. Not controlling copyability and assignability.
You have an object in a function which has local scope. You then pass a
pointer to some code that runs independent of that scope and that code (in
this case a different thread) keeps a copy of that pointer, long past the
time that the object lives. This is mainly a manifestation of unclear
ownership, because the only owner in your code is the function where the
local resides, but the thread assumes a shared ownership.
There are two ways to resolve this:
1. exclusive ownership
2. shared ownership
Exclusive ownership means you simply move the object to the thread so it
really is detached from the initiating thread. Shared ownership could mean
that both store a pointer, but they have to coordinate who destroys the
object. Use std::auto_ptr<> for exclusive ownership and boost::shared_ptr<>
for shared ownership.
Now, the second option would be to give the thread a copy of the object, so
that both have independant instances. You could try this with your classes
and it would compile, but it wouldn't work. The reason is that while the
resources/entities your classes represent are not copyable, the
pointers/handles they contain are, and the compiler generates a
copy-constructor and assignment operator that simply copies the
pointers/handles. You need to prevent this unless you specifically cater
for it, do a websearch for the "Law of Three" and "C++" and see the FAQ at
parashift's.
Obeying those rules, the beast C++ can be tamed and it would have kept your
code from compiling instead of running incorrectly.
Uli
|
|
0
|
|
|
|
Reply
|
Ulrich
|
6/22/2007 6:07:16 PM
|
|
> // Terminate this thread
> // Note that the calling thread DOES NOT return from this call
> void EndThread(void);
This seems a little wrong to me, apart from all the other things
mentioned. Ending the thread should not end the calling thread, but
encapsulated thread. Therefore you should consider joining perhaps, or
a combination of cancellation or joining, depending on how the thread
was created.
|
|
0
|
|
|
|
Reply
|
werasm
|
6/29/2007 1:17:44 PM
|
|
|
5 Replies
630 Views
(page loaded in 0.08 seconds)
|