r3128 - in trunk/libmutil: include/libmutil source
erik at minisip.org
erik at minisip.org
Fri Jan 19 16:55:51 CET 2007
Author: erik
Date: 2007-01-19 16:55:50 +0100 (Fri, 19 Jan 2007)
New Revision: 3128
Modified:
trunk/libmutil/include/libmutil/Thread.h
trunk/libmutil/source/ThreadPosix.cxx
trunk/libmutil/source/ThreadWin32.cxx
Log:
* Short: Changed the internals (in a sub-optimal way) of ThreadHandle
to work around a bug.
I have had serious trouble with a strange bug when using threads
with ThreadHandle (as opposed to using Thread objects).
The correct thread handle has not been stored in the object
returned by Thread::createThread(f,arg). It has looked exactly
like the copy constructor has not been called (...and no, it's
not the optimization by GCC that writes the return value directly
into the callers variable that is sometimes confusing).
I get a duplicate free of the dynamically allocated pointer in
ThreadHandle. Valgrind also says that value pointed to by
the pointer in ThreadHandle is not initialized.
It feels just like some simple problem, but I don't see what it
is. This commit changes the internals of ThreadHandle so that
it does not dynamically allocate data on the heap. This way
there is no problem for me. This is not perfect though - it stores
the handle in an unsigned long. This is a problem when pointers
are 64 bit and long is 32. That is true for the LLP64 model used
in 64 bit MS Windows, but not in the LP64 model used in Linux et.al.
It's also bad because it assumes that a pthread_t is an unsigned
long, and that HANDLE is a void *.
Not very good, but something needs to be done to go forward. Maybe
someone sees what I missed or it's a wierd side effect of something
else.
* Fixed const-ness of ThreadHandle::asLongInt()
Modified: trunk/libmutil/include/libmutil/Thread.h
===================================================================
--- trunk/libmutil/include/libmutil/Thread.h 2007-01-19 15:43:15 UTC (rev 3127)
+++ trunk/libmutil/include/libmutil/Thread.h 2007-01-19 15:55:50 UTC (rev 3128)
@@ -49,12 +49,15 @@
ThreadHandle(const ThreadHandle &);
~ThreadHandle();
- unsigned long asLongInt() {
- return *((unsigned long*)this->hptr);
+ unsigned long asLongInt() const{
+ //return *((unsigned long*)this->hptr);
+ return handle;
};
private:
- void *hptr;
+ //void *hptr;
+ unsigned long handle;
+
friend class Thread;
};
@@ -129,7 +132,6 @@
private:
ThreadHandle handle;
-// void *handle_ptr;
};
Modified: trunk/libmutil/source/ThreadPosix.cxx
===================================================================
--- trunk/libmutil/source/ThreadPosix.cxx 2007-01-19 15:43:15 UTC (rev 3127)
+++ trunk/libmutil/source/ThreadPosix.cxx 2007-01-19 15:55:50 UTC (rev 3128)
@@ -312,8 +312,8 @@
*/
ret = pthread_create(
- //(pthread_t*)handle_ptr,
- (pthread_t*)handle.hptr,
+ //(pthread_t*)handle.hptr,
+ (pthread_t*)&handle.handle,
NULL, // either NULL or &attr,
LinuxThreadStarter,
self);
@@ -344,12 +344,12 @@
ThreadHandle Thread::createThread(void f()){
//pthread_t threadHandle;
- ThreadHandle handle;
+ ThreadHandle h;
#ifdef DEBUG_OUTPUT
mdbg << "Running createThread"<< end;
#endif // DEBUG_OUTPUT
- pthread_create((pthread_t*)handle.hptr, NULL, LinuxStaticThreadStarter, (void*)f);
- return handle;
+ pthread_create( /* (pthread_t*)h.hptr*/ (pthread_t*)&h.handle, NULL, LinuxStaticThreadStarter, (void*)f);
+ return h;
}
ThreadHandle Thread::createThread(void *f(void*), void *arg){
@@ -357,13 +357,12 @@
argptr->fun = (void*)f;
argptr->arg = arg;
- //pthread_t threadHandle;
- ThreadHandle handle;
+ ThreadHandle h;
#ifdef DEBUG_OUTPUT
mdbg << "Running createThread" << end;
#endif // DEBUG_OUTPUT
- pthread_create((pthread_t*)handle.hptr, NULL, LinuxStaticThreadStarterArg, argptr);
- return handle;
+ pthread_create(/* (pthread_t*)h.hptr*/ (pthread_t*)&h.handle, NULL, LinuxStaticThreadStarterArg, argptr);
+ return h;
}
void * Thread::join(){
@@ -374,8 +373,8 @@
mdbg << "Thread::join(): before join" << end;
#endif // DEBUG_OUTPUT
ret = pthread_join(
- // *( (pthread_t *)handle_ptr ),
- *((pthread_t *)handle.hptr),
+ //*((pthread_t *)handle.hptr),
+ (pthread_t)handle.handle,
&returnValue );
if( ret != 0 ){
@@ -389,8 +388,8 @@
return returnValue;
}
-void Thread::join(const ThreadHandle &handle){
- if( pthread_join( *((pthread_t*)handle.hptr), NULL) ){
+void Thread::join(const ThreadHandle &h){
+ if( pthread_join( /* *((pthread_t*)h.hptr)*/ (pthread_t)h.handle, NULL) ){
#ifdef DEBUG_OUTPUT
merror("Thread::join: pthread_join");
#endif
@@ -408,8 +407,8 @@
#ifdef DEBUG_OUTPUT
mdbg << "Thread::kill(): before cancel" << end;
#endif // DEBUG_OUTPUT
- //ret = pthread_cancel( *( (pthread_t *)handle_ptr ) );
- ret = pthread_cancel( *( (pthread_t *)handle.hptr) );
+ //ret = pthread_cancel( *( (pthread_t *)handle) );
+ ret = pthread_cancel( /* *( (pthread_t *)handle.hptr)*/ (pthread_t)handle.handle );
if( ret != 0 ){
#ifdef DEBUG_OUTPUT
@@ -421,13 +420,13 @@
return true;
}
-bool Thread::kill( const ThreadHandle &handle) {
+bool Thread::kill( const ThreadHandle &h) {
int ret;
#ifdef DEBUG_OUTPUT
mdbg << "Thread::kill(): before cancel" << end;
#endif // DEBUG_OUTPUT
- ret = pthread_cancel( *((pthread_t*)handle.hptr) );
+ ret = pthread_cancel( /* *((pthread_t*)h.hptr) */ (pthread_t)h.handle );
if( ret != 0 ){
#ifdef DEBUG_OUTPUT
@@ -441,22 +440,25 @@
ThreadHandle Thread::getCurrent() {
ThreadHandle th;
- *((pthread_t*)th.hptr) = pthread_self();
+ //*((pthread_t*)th.hptr) = pthread_self();
+ th.handle = pthread_self();
return th;
}
ThreadHandle::ThreadHandle(){
- hptr = (void*)new pthread_t;
+// hptr = (void*)new pthread_t;
+ handle=0;
}
ThreadHandle::~ThreadHandle(){
- delete (pthread_t*)hptr;
- hptr=NULL;
+// delete (pthread_t*)hptr;
+// hptr=NULL;
}
ThreadHandle::ThreadHandle(const ThreadHandle &h){
- hptr = (void*)new pthread_t;
- *((pthread_t*)hptr)= *((pthread_t*)h.hptr);
+ //hptr = (void*)new pthread_t;
+ //*((pthread_t*)hptr)= *((pthread_t*)h.hptr);
+ handle = h.handle;
}
Modified: trunk/libmutil/source/ThreadWin32.cxx
===================================================================
--- trunk/libmutil/source/ThreadWin32.cxx 2007-01-19 15:43:15 UTC (rev 3127)
+++ trunk/libmutil/source/ThreadWin32.cxx 2007-01-19 15:55:50 UTC (rev 3128)
@@ -160,8 +160,8 @@
delete self;
throw ThreadException("Could not create thread.");
}
- *((HANDLE*)handle.hptr) = h;
-// printf("In Thread, windows part - thread created\n");
+ //*((HANDLE*)handle.hptr) = h;
+ handle.handle = (unsigned long)h;
}
@@ -186,7 +186,9 @@
merror("Thread::Thread: CreateThread");
throw ThreadException("Could not create thread.");
}
- *((HANDLE*)handle.hptr) = h;
+ //*((HANDLE*)handle.hptr) = h;
+ handle.handle = (unsigned long)h;
+
return handle;
}
@@ -219,7 +221,8 @@
merror("Thread::createThread: CreateThread");
throw ThreadException("Could not create thread.");
}
- *((HANDLE*)handle.hptr) = h;
+ //*((HANDLE*)handle.hptr) = h;
+ handle.handle=(unsigned long)h;
return handle;
}
@@ -229,7 +232,8 @@
}
void Thread::join(const ThreadHandle &handle){
- HANDLE h = *((HANDLE*)handle.hptr);
+ //HANDLE h = *((HANDLE*)handle.hptr);
+ HANDLE h = (HANDLE)handle.handle;
if (WaitForSingleObject( h, INFINITE )==WAIT_FAILED){
merror("Thread::join:WaitForSingleObject");
}
@@ -245,7 +249,8 @@
}
bool Thread::kill( const ThreadHandle &handle) {
- HANDLE h = *((HANDLE*)handle.hptr);
+ //HANDLE h = *((HANDLE*)handle.hptr);
+ HANDLE h = (HANDLE)handle.handle;
BOOL ret;
DWORD lpExitCode;
@@ -267,17 +272,19 @@
}
ThreadHandle::ThreadHandle(){
- hptr = (void*) new HANDLE;
+ //hptr = (void*) new HANDLE;
+ handle=0;
}
ThreadHandle::~ThreadHandle(){
- delete (HANDLE*)hptr;
- hptr=NULL;
+// delete (HANDLE*)hptr;
+// hptr=NULL;
}
ThreadHandle::ThreadHandle(const ThreadHandle &h){
- hptr = (void*)new HANDLE;
- *((HANDLE*)hptr)= *((HANDLE*)h.hptr);
+ //hptr = (void*)new HANDLE;
+ //*((HANDLE*)hptr)= *((HANDLE*)h.hptr);
+ handle=h.handle;
}
More information about the Minisip-devel
mailing list