Fixed bugs 724 and 725

1 files changed, 105 insertions, 223 deletions

diff --git a/cpp/src/IceUtil/Thread.cpp b/cpp/src/IceUtil/Thread.cpp
index 65e1886a7b8..8c10fbe8475 100644
--- a/cpp/src/IceUtil/Thread.cpp
+++ b/cpp/src/IceUtil/Thread.cpp
@@ -15,32 +15,53 @@ using namespace std;
 
 #ifdef _WIN32
 
-IceUtil::ThreadControl::ThreadControl()
-{
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _handle = new HandleWrapper(GetCurrentThread(), false);
-    _id = GetCurrentThreadId();
+IceUtil::ThreadControl::ThreadControl() :
+    _handle(0),
+    _id(GetCurrentThreadId())
+{
+    HANDLE currentThread = GetCurrentThread();
+    HANDLE currentProcess = GetCurrentProcess();   
+
+    if(DuplicateHandle(currentProcess,
+		       currentThread,
+		       currentProcess,
+		       &_handle,
+		       0,
+		       FALSE,
+		       DUPLICATE_SAME_ACCESS) == 0)
+    {
+	throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
+    }
 }
 
-IceUtil::ThreadControl::ThreadControl(const HandleWrapperPtr& handle, ThreadId id)
+IceUtil::ThreadControl::ThreadControl(HANDLE handle, DWORD id) :
+    _id(id)
 {
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _handle = handle;
-    _id = id;
-}
+    HANDLE currentProcess = GetCurrentProcess();    
 
-IceUtil::ThreadControl::ThreadControl(const ThreadControl& tc)
-{
-    ThreadId id;
-    HandleWrapperPtr handle;
+    if(DuplicateHandle(currentProcess,
+		       handle,
+		       currentProcess,
+		       &_handle,
+		       0,
+		       FALSE,
+		       DUPLICATE_SAME_ACCESS) == 0)
     {
-	IceUtil::Mutex::Lock lock(tc._stateMutex);
-	id = tc._id;
-	handle = tc._handle;
+	throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
     }
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _handle = handle;
-    _id = id;
+}
+  
+IceUtil::ThreadControl::ThreadControl(const ThreadControl& tc) :
+    _handle(0)
+{
+    operator=(tc);
+}
+
+IceUtil::ThreadControl::~ThreadControl()
+{
+    assert(_handle != 0);
+    bool ok = CloseHandle(_handle);
+    assert(ok);
 }
 
 IceUtil::ThreadControl&
@@ -48,16 +69,23 @@ IceUtil::ThreadControl::operator=(const ThreadControl& rhs)
 {
     if(&rhs != this)
     {
-	ThreadId id;
-	HandleWrapperPtr handle;
+	if(_handle != 0 && CloseHandle(_handle) == 0)
 	{
-	    IceUtil::Mutex::Lock lock(rhs._stateMutex);
-	    handle = rhs._handle;
-	    id = rhs._id;
+	    throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
+	}	
+	_id = rhs._id;
+
+	HANDLE currentProcess = GetCurrentProcess(); 
+	if(DuplicateHandle(currentProcess,
+			   rhs._handle,
+			   currentProcess,
+			   &_handle,
+			   0,
+			   FALSE,
+			   DUPLICATE_SAME_ACCESS) == 0)
+	{
+	    throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
 	}
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	_handle = handle;
-	_id = id;
     }
     return *this;
 }
@@ -65,41 +93,20 @@ IceUtil::ThreadControl::operator=(const ThreadControl& rhs)
 bool
 IceUtil::ThreadControl::operator==(const ThreadControl& rhs) const
 {
-    ThreadId id = rhs.id();
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    return _id == id;
+    return _id == rhs._id;
 }
 
 bool
 IceUtil::ThreadControl::operator!=(const ThreadControl& rhs) const
 {
-    return !operator==(rhs);
-}
-
-bool
-IceUtil::ThreadControl::operator<(const ThreadControl& rhs) const
-{
-    ThreadId id = rhs.id();
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    return _id < id;
+    return _id != rhs._id;
 }
 
-IceUtil::ThreadId
-IceUtil::ThreadControl::id() const
-{
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    return _id;
-}
 
 void
 IceUtil::ThreadControl::join()
 {
-    HandleWrapperPtr handle;
-    {
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	handle = _handle;
-    }
-    int rc = WaitForSingleObject(handle->handle, INFINITE);
+    int rc = WaitForSingleObject(_handle, INFINITE);
     if(rc != WAIT_OBJECT_0)
     {
 	throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
@@ -109,19 +116,15 @@ IceUtil::ThreadControl::join()
 void
 IceUtil::ThreadControl::detach()
 {
-    // No-op: Windows doesn't have the concept of detaching a thread.
+    // On Windows, a thread is cleaned up when the last handle to this thread
+    // is closed.
 }
 
 bool
 IceUtil::ThreadControl::isAlive() const
 {
-    HandleWrapperPtr handle;
-    {
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	handle = _handle;
-    }
     DWORD rc;
-    if(GetExitCodeThread(handle->handle, &rc) == 0)
+    if(GetExitCodeThread(_handle, &rc) == 0)
     {
 	return false;
     }
@@ -145,27 +148,20 @@ IceUtil::ThreadControl::yield()
     Sleep(0);
 }
 
-IceUtil::Thread::Thread()
+IceUtil::Thread::Thread() :
+    _started(false),
+    _handle(0),
+    _id(0)
 {
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _started = false;
-    _id = 0;
-    _handle = new HandleWrapper(0);
 }
 
 IceUtil::Thread::~Thread()
 {
-}
-
-IceUtil::ThreadId
-IceUtil::Thread::id() const
-{
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
+    if(_handle != 0)
     {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
+	bool ok = CloseHandle(_handle);
+	assert(ok);
     }
-    return _id;
 }
 
 static unsigned int
@@ -229,13 +225,16 @@ IceUtil::Thread::start(size_t stackSize)
     //
     __incRef();
     
-    _handle->handle = 
+    unsigned int id;
+    
+    _handle = 
        reinterpret_cast<HANDLE>(
           _beginthreadex(0, 
 			 static_cast<unsigned int>(stackSize), 
-			 startHook, this, 0, &_id));
+			 startHook, this, 0, &id));
+    _id = id;
 
-    if(_handle->handle == 0)
+    if(_handle == 0)
     {
 	__decRef();
 	throw ThreadSyscallException(__FILE__, __LINE__, GetLastError());
@@ -260,76 +259,40 @@ IceUtil::Thread::getThreadControl() const
 bool
 IceUtil::Thread::operator==(const Thread& rhs) const
 {
-    //
-    // Get rhs ID.
-    //
-    ThreadId id = rhs.id();
-
-    //
-    // Check that this thread was started.
-    //
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
-    {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
-    }
-
-    //
-    // We perform the comparison within the scope of the lock, otherwise the hardware has no
-    // way of knowing that it might have to flush a cache line.
-    //
-    return _id == id;
+    return this == &rhs;
 }
 
 bool
 IceUtil::Thread::operator!=(const Thread& rhs) const
 {
-    return !operator==(rhs);
+    return this != &rhs;
 }
 
 bool
 IceUtil::Thread::operator<(const Thread& rhs) const
 {
-    //
-    // Get rhs ID.
-    //
-    ThreadId id = rhs.id();
-
-    //
-    // Check that this thread was started.
-    //
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
-    {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
-    }
-
-    //
-    // We perform the comparison within the scope of the lock, otherwise the hardware has no
-    // way of knowing that it might have to flush a cache line.
-    //
-    return _id < id;
+    return this < &rhs;
 }
 
 #else
 
-IceUtil::ThreadControl::ThreadControl(ThreadId id)
+IceUtil::ThreadControl::ThreadControl(pthread_t thread) :
+    _thread(thread)
 {
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _id = id;
 }
 
-IceUtil::ThreadControl::ThreadControl()
+IceUtil::ThreadControl::ThreadControl() :
+    _thread(pthread_self())
 {
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _id = pthread_self();
 }
 
-IceUtil::ThreadControl::ThreadControl(const ThreadControl& tc)
+IceUtil::ThreadControl::ThreadControl(const ThreadControl& tc) :
+    _thread(tc._thread)
+{
+}
+
+IceUtil::ThreadControl::~ThreadControl()
 {
-    ThreadId id = tc.id();
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _id = id;
 }
 
 IceUtil::ThreadControl&
@@ -337,9 +300,7 @@ IceUtil::ThreadControl::operator=(const ThreadControl& rhs)
 {
     if(&rhs != this)
     {
-	ThreadId id = rhs.id();
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	_id = id;
+	_thread = rhs._thread;
     }
     return *this;
 }
@@ -347,9 +308,7 @@ IceUtil::ThreadControl::operator=(const ThreadControl& rhs)
 bool
 IceUtil::ThreadControl::operator==(const ThreadControl& rhs) const
 {
-    ThreadId id = rhs.id();
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    return pthread_equal(_id, id);
+    return pthread_equal(_thread, rhs._thread) != 0;
 }
 
 bool
@@ -358,32 +317,11 @@ IceUtil::ThreadControl::operator!=(const ThreadControl& rhs) const
     return !operator==(rhs);
 }
 
-bool
-IceUtil::ThreadControl::operator<(const ThreadControl& rhs) const
-{
-    ThreadId id = rhs.id();
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    // NOTE: Linux specific
-    return _id < id;
-}
-
-IceUtil::ThreadId
-IceUtil::ThreadControl::id() const
-{
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    return _id;
-}
-
 void
 IceUtil::ThreadControl::join()
 {
-    ThreadId id;
-    {
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	id = _id;
-    }
     void* ignore = 0;
-    int rc = pthread_join(id, &ignore);
+    int rc = pthread_join(_thread, &ignore);
     if(rc != 0)
     {
 	throw ThreadSyscallException(__FILE__, __LINE__, rc);
@@ -393,12 +331,7 @@ IceUtil::ThreadControl::join()
 void
 IceUtil::ThreadControl::detach()
 {
-    ThreadId id;
-    {
-	IceUtil::Mutex::Lock lock(_stateMutex);
-	id = _id;
-    }
-    int rc = pthread_detach(id);
+    int rc = pthread_detach(_thread);
     if(rc != 0)
     {
 	throw ThreadSyscallException(__FILE__, __LINE__, rc);
@@ -411,13 +344,11 @@ IceUtil::ThreadControl::isAlive() const
     int policy;
     int ret;
     struct sched_param param;
-    IceUtil::Mutex::Lock lock(_stateMutex);
-
-    ret = pthread_getschedparam(_id, &policy, &param);
+    ret = pthread_getschedparam(_thread, &policy, &param);
 #ifdef __APPLE__
     if (ret == 0) 
     {
-	ret = pthread_setschedparam(_id, policy, &param);
+	ret = pthread_setschedparam(_thread, policy, &param);
     }
 #endif 
     return (ret == 0);
@@ -439,29 +370,18 @@ IceUtil::ThreadControl::yield()
     sched_yield();
 }
 
-IceUtil::Thread::Thread()
+IceUtil::Thread::Thread() :
+    _started(false)
 {
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    _started = false;
-    _id = 0;
 }
 
 IceUtil::Thread::~Thread()
 {
 }
 
-IceUtil::ThreadId
-IceUtil::Thread::id() const
-{
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
-    {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
-    }
-    return _id;
-}
 
-extern "C" {
+extern "C" 
+{
 static void*
 startHook(void* arg)
 {
@@ -535,7 +455,7 @@ IceUtil::Thread::start(size_t stackSize)
 	    __decRef();
 	    throw ThreadSyscallException(__FILE__, __LINE__, rc);
 	}
-	rc = pthread_create(&_id, &attr, startHook, this);
+	rc = pthread_create(&_thread, &attr, startHook, this);
 	if(rc != 0)
 	{
 	    __decRef();
@@ -544,7 +464,7 @@ IceUtil::Thread::start(size_t stackSize)
     }
     else
     {
-	int rc = pthread_create(&_id, 0, startHook, this);
+	int rc = pthread_create(&_thread, 0, startHook, this);
 	if(rc != 0)
 	{
 	    __decRef();
@@ -554,7 +474,7 @@ IceUtil::Thread::start(size_t stackSize)
 
     _started = true;
 
-    return ThreadControl(_id);
+    return ThreadControl(_thread);
 }
 
 IceUtil::ThreadControl
@@ -565,63 +485,25 @@ IceUtil::Thread::getThreadControl() const
     {
 	throw ThreadNotStartedException(__FILE__, __LINE__);
     }
-    return ThreadControl(_id);
+    return ThreadControl(_thread);
 }
 
 bool
 IceUtil::Thread::operator==(const Thread& rhs) const
 {
-    //
-    // Get rhs ID.
-    //
-    ThreadId id = rhs.id();
-
-    //
-    // Check that this thread was started.
-    //
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
-    {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
-    }
-
-    //
-    // We perform the comparison within the scope of the lock, otherwise the hardware has no
-    // way of knowing that it might have to flush a cache line.
-    //
-    return pthread_equal(_id, id);
+    return this == &rhs;
 }
 
 bool
 IceUtil::Thread::operator!=(const Thread& rhs) const
 {
-    return !operator==(rhs);
+    return this != &rhs;
 }
 
 bool
 IceUtil::Thread::operator<(const Thread& rhs) const
 {
-    //
-    // Get rhs ID.
-    //
-    ThreadId id = rhs.id();
-
-    //
-    // Check that this thread was started.
-    //
-    IceUtil::Mutex::Lock lock(_stateMutex);
-    if(!_started)
-    {
-	throw ThreadNotStartedException(__FILE__, __LINE__);
-    }
-
-    //
-    // We perform the comparison within the scope of the lock, otherwise the hardware has no
-    // way of knowing that it might have to flush a cache line.
-    //
-    // NOTE: Linux specific
-    //
-    return _id < id;
+    return this < &rhs;
 }
 
 #endif