I have discovered some problems with the current MTinterface
implementation. Here are 2 test cases:
1: mainthread related
#include <stdio.h>
#include <unistd.h>
#include <pthread.h>
static void * Thread (void *);
static pthread_t main_thread;
int main(void)
{
pthread_t t;
main_thread = pthread_self ();
pthread_create (&t, NULL, Thread, NULL);
sleep (5);
pthread_exit (t);
return 0;
}
static void * Thread (void *not_used)
{
pthread_join (main_thread, NULL);
return NULL;
}
This valid code doesn't work at all. The mainthread object in MTinterface
is not properly initialized, the cancel_event is NULL and the win32_obj_id
is NULL because myself->hProcess is NULL when MTinterface is initialized
(and i don't think that a process handle can be used as thread handle).
Even if the handles would be valid the pthread_join call would try to
delete a thread object that is created static which would result in a
corrupted heap.
2: fork related
#include <stdio.h>
#include <unistd.h>
#include <sys/wait.h>
#include <pthread.h>
static void * TestThread ( void * );
int main (void)
{
pthread_t t;
printf ("main: %p\n", pthread_self ());
pthread_create (&t, NULL, TestThread, NULL);
pthread_join (t, NULL);
return 0;
}
static void * TestThread ( void *not_used )
{
switch (fork ())
{
case -1:
return NULL;
case 0:
printf ("child: %p\n", pthread_self());
break;
default:
printf ("parent: %p\n", pthread_self());
wait (NULL);
}
return NULL;
}
The forked child will not get the same thread handle as its parent, it
will get the thread handle from the main thread instead. The child will
not terminate because the threadcount is still 2 after the fork (it is
set to 1 in MTinterface::Init and then set back to 2 after the childs
memory gets overwritten by the parent).
And i do not agree with the the current pthread_self code where the
threadcount is incremented if a new thread handle has been created but
never gets decremented (i do not expect that threads that are not created
by pthread_created will terminate via pthread_exit). And the newly created
object never gets freed.
To avoid these errors i have made changes that will create the mainthread
object dynamic and store the reents and thread self pointer via fork safe
keys.
Thomas
2002-11-05 Thomas Pfaff <[EMAIL PROTECTED]>
* dcrt0.cc (dll_crt0_1): Add call to pthread::initMainThread to
initialize mainthread when it is safe to call new.
* init.cc (dll_entry): Change call to store reents in tls key.
* thread.cc (_reent_clib) : Change call to get reents from tls
key.
(_reent_winsup): Ditto.
(MTinterface::Init): Key handling changed. Remove initialization
of member variables.
(MTinterface::fixup_after_fork): Reinitialize mainthread object
after fork. Reset threadount to 1.
(pthread::initMainThread): Create mainthread object dynamically.
and initialize with valid handles.
(pthread::self): Remove calls to create thread objects.
(pthread::setTlsSelfPointer): Change call to store thread self
handle in tls key.
(pthread::getTlsSelfPointer): New static method.
(pthread::exit): Remove setTlsSelfPointer call.
(pthread::initCurrentThread): New method.
(pthread::thread_init_wrapper): Change call to store thread self
handle in tls key.
(pthread::join): Check for a valid joiner.
(pthreadNull::pthreadNull): Mark Null object as detached.
(pthreadNull::exit): Terminate thread via ExitThread.
* thread.h (pthread::initMainThread): Change parameter in function
call.
(pthread::getTlsSelfPointer): New static method.
(pthread::initCurrentThread): New method.
(MTinterface::reent_key): Remove.
(MTinterface::thread_self_dwTlsIndex): Ditto..
(MTinterface::indexallocated): Ditto.
(MTinterface::mainthread): Ditto.
(MTinterface::reent_key): New member.
(MTinterface::thread_self_key): Ditto.
(MTinterface::MTinterface): Initialize all members.
diff -urp src.old/winsup/cygwin/dcrt0.cc src/winsup/cygwin/dcrt0.cc
--- src.old/winsup/cygwin/dcrt0.cc 2002-10-23 20:42:51.000000000 +0200
+++ src/winsup/cygwin/dcrt0.cc 2002-10-31 10:22:17.000000000 +0100
@@ -628,6 +628,8 @@ dll_crt0_1 ()
ProtectHandle (hMainThread);
cygthread::init ();
+ pthread::initMainThread (!user_data->forkee);
+
/* Initialize debug muto, if DLL is built with --enable-debugging.
Need to do this before any helper threads start. */
debug_init ();
diff -urp src.old/winsup/cygwin/init.cc src/winsup/cygwin/init.cc
--- src.old/winsup/cygwin/init.cc 2002-10-13 20:31:18.000000000 +0200
+++ src/winsup/cygwin/init.cc 2002-10-28 16:28:58.000000000 +0100
@@ -27,12 +27,8 @@ WINAPI dll_entry (HANDLE h, DWORD reason
case DLL_PROCESS_DETACH:
break;
case DLL_THREAD_ATTACH:
- if (user_data->threadinterface)
- {
- if (!TlsSetValue (user_data->threadinterface->reent_index,
- &user_data->threadinterface->reents))
+ if (MT_INTERFACE->reent_key.set (&MT_INTERFACE->reents))
api_fatal ("thread initialization failed");
- }
break;
case DLL_THREAD_DETACH:
/* not invoked */;
diff -urp src.old/winsup/cygwin/thread.cc src/winsup/cygwin/thread.cc
--- src.old/winsup/cygwin/thread.cc 2002-10-21 03:03:32.000000000 +0200
+++ src/winsup/cygwin/thread.cc 2002-11-05 13:16:18.000000000 +0100
@@ -46,35 +46,29 @@ details. */
extern int threadsafe;
-#define MT_INTERFACE user_data->threadinterface
-
struct _reent *
_reent_clib ()
{
- int tmp = GetLastError ();
struct __reent_t *_r =
- (struct __reent_t *) TlsGetValue (MT_INTERFACE->reent_index);
+ (struct __reent_t *) MT_INTERFACE->reent_key.get ();
#ifdef _CYG_THREAD_FAILSAFE
if (_r == 0)
system_printf ("local thread storage not inited");
#endif
-
- SetLastError (tmp);
return _r->_clib;
}
struct _winsup_t *
_reent_winsup ()
{
- int tmp = GetLastError ();
- struct __reent_t *_r;
- _r = (struct __reent_t *) TlsGetValue (MT_INTERFACE->reent_index);
+ struct __reent_t *_r =
+ (struct __reent_t *) MT_INTERFACE->reent_key.get ();
+
#ifdef _CYG_THREAD_FAILSAFE
if (_r == 0)
system_printf ("local thread storage not inited");
#endif
- SetLastError (tmp);
return _r->_winsup;
}
@@ -166,39 +160,14 @@ ResourceLocks::Delete ()
void
MTinterface::Init (int forked)
{
-
- reent_index = TlsAlloc ();
reents._clib = _impure_ptr;
reents._winsup = &winsup_reent;
-
winsup_reent._process_logmask = LOG_UPTO (LOG_DEBUG);
- TlsSetValue (reent_index, &reents);
- // the static reent_data will be used in the main thread
-
- if (!indexallocated)
- {
- thread_self_dwTlsIndex = TlsAlloc ();
- if (thread_self_dwTlsIndex == TLS_OUT_OF_INDEXES)
- system_printf
- ("local storage for thread couldn't be set\nThis means that we are not
thread safe!");
- else
- indexallocated = (-1);
- }
+ if (!forked)
+ reent_key.set (&reents);
- concurrency = 0;
- threadcount = 1; /* 1 current thread when Init occurs.*/
-
- pthread::initMainThread (&mainthread, myself->hProcess);
pthread_mutex::initMutex ();
-
- if (forked)
- return;
-
- mutexs = NULL;
- conds = NULL;
- semaphores = NULL;
-
}
void
@@ -233,40 +202,51 @@ MTinterface::fixup_after_fork (void)
sem->fixup_after_fork ();
sem = sem->next;
}
+
+ pthread::initMainThread (true);
+
+ threadcount = 1;
}
/* pthread calls */
/* static methods */
void
-pthread::initMainThread (pthread *mainThread, HANDLE win32_obj_id)
+pthread::initMainThread (bool do_init)
{
- mainThread->win32_obj_id = win32_obj_id;
- mainThread->setThreadIdtoCurrent ();
- setTlsSelfPointer (mainThread);
+ if (!do_init)
+ return;
+
+ pthread *thread = getTlsSelfPointer ();
+ if (!thread)
+ {
+ thread = new pthread ();
+ if (!thread)
+ api_fatal ("failed to create mainthread object");
+ }
+
+ thread->initCurrentThread ();
}
pthread *
pthread::self ()
{
- pthread *temp = (pthread *) TlsGetValue (MT_INTERFACE->thread_self_dwTlsIndex);
- if (temp)
- return temp;
- temp = new pthread ();
- temp->precreate (NULL);
- if (!temp->magic) {
- delete temp;
- return pthreadNull::getNullpthread ();
- }
- temp->postcreate ();
- return temp;
+ pthread *thread = getTlsSelfPointer ();
+ if (thread)
+ return thread;
+ return pthreadNull::getNullpthread ();
}
void
pthread::setTlsSelfPointer (pthread *thisThread)
{
- /* the OS doesn't check this for <= 64 Tls entries (pre win2k) */
- TlsSetValue (MT_INTERFACE->thread_self_dwTlsIndex, thisThread);
+ MT_INTERFACE->thread_self_key.set (thisThread);
+}
+
+pthread *
+pthread::getTlsSelfPointer ()
+{
+ return (pthread *) MT_INTERFACE->thread_self_key.get ();
}
@@ -384,9 +364,6 @@ pthread::exit (void *value_ptr)
mutex.UnLock ();
}
- /* Prevent DLL_THREAD_DETACH Attempting to clean us up */
- setTlsSelfPointer (0);
-
if (InterlockedDecrement (&MT_INTERFACE->threadcount) == 0)
::exit (0);
else
@@ -715,6 +692,18 @@ pthread::getThreadId ()
return thread_id;
}
+void
+pthread::initCurrentThread ()
+{
+ cancel_event = ::CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+ if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (),
+ GetCurrentProcess (), &win32_obj_id,
+ 0, FALSE, DUPLICATE_SAME_ACCESS))
+ win32_obj_id = NULL;
+ setThreadIdtoCurrent ();
+ setTlsSelfPointer (this);
+}
+
/* static members */
bool
pthread_attr::isGoodObject (pthread_attr_t const *attr)
@@ -1411,16 +1400,15 @@ pthread::thread_init_wrapper (void *_arg
local_winsup._process_logmask = LOG_UPTO (LOG_DEBUG);
- /* This is not checked by the OS !! */
- if (!TlsSetValue (MT_INTERFACE->reent_index, &local_reent))
- system_printf ("local storage for thread couldn't be set");
+ MT_INTERFACE->reent_key.set (&local_reent);
+ thread->setThreadIdtoCurrent ();
setTlsSelfPointer (thread);
thread->mutex.Lock ();
// if thread is detached force cleanup on exit
if (thread->attr.joinable == PTHREAD_CREATE_DETACHED && thread->joiner == NULL)
- thread->joiner = pthread::self ();
+ thread->joiner = thread;
thread->mutex.UnLock ();
#ifdef _CYG_THREAD_FAILSAFE
@@ -1787,6 +1775,9 @@ pthread::join (pthread_t *thread, void *
{
pthread_t joiner = self ();
+ if (!isGoodObject (&joiner))
+ return EINVAL;
+
// Initialize return val with NULL
if (return_val)
*return_val = NULL;
@@ -2594,6 +2585,7 @@ pthreadNull::getNullpthread ()
pthreadNull::pthreadNull ()
{
+ attr.joinable = PTHREAD_CREATE_DETACHED;
/* Mark ourselves as invalid */
magic = 0;
}
@@ -2610,6 +2602,7 @@ pthreadNull::create (void *(*)(void *),
void
pthreadNull::exit (void *value_ptr)
{
+ ExitThread (0);
}
int
diff -urp src.old/winsup/cygwin/thread.h src/winsup/cygwin/thread.h
--- src.old/winsup/cygwin/thread.h 2002-10-17 23:38:06.000000000 +0200
+++ src/winsup/cygwin/thread.h 2002-11-04 10:08:07.000000000 +0100
@@ -344,7 +344,7 @@ public:
pthread ();
virtual ~pthread ();
- static void initMainThread(pthread *, HANDLE);
+ static void initMainThread (bool);
static bool isGoodObject(pthread_t const *);
static void atforkprepare();
static void atforkparent();
@@ -387,10 +387,12 @@ private:
void pop_all_cleanup_handlers (void);
void precreate (pthread_attr *);
void postcreate ();
- void setThreadIdtoCurrent();
- static void setTlsSelfPointer(pthread *);
+ void setThreadIdtoCurrent ();
+ static void setTlsSelfPointer (pthread *);
+ static pthread *getTlsSelfPointer ();
void cancel_self ();
DWORD getThreadId ();
+ void initCurrentThread ();
};
class pthreadNull : public pthread
@@ -493,17 +495,12 @@ class MTinterface
{
public:
// General
- DWORD reent_index;
- DWORD thread_self_dwTlsIndex;
- /* we may get 0 for the Tls index.. grrr */
- int indexallocated;
int concurrency;
long int threadcount;
// Used for main thread data, and sigproc thread
struct __reent_t reents;
struct _winsup_t winsup_reent;
- pthread mainthread;
callback *pthread_prepare;
callback *pthread_child;
@@ -514,18 +511,25 @@ public:
class pthread_cond * conds;
class semaphore * semaphores;
+ pthread_key reent_key;
+ pthread_key thread_self_key;
+
void Init (int);
void fixup_before_fork (void);
void fixup_after_fork (void);
- MTinterface ():reent_index (0), indexallocated (0), threadcount (1)
+ MTinterface () :
+ concurrency (0), threadcount (1),
+ pthread_prepare (NULL), pthread_child (NULL), pthread_parent (NULL),
+ mutexs (NULL), conds (NULL), semaphores (NULL),
+ reent_key (NULL), thread_self_key (NULL)
{
- pthread_prepare = NULL;
- pthread_child = NULL;
- pthread_parent = NULL;
}
};
+
+#define MT_INTERFACE user_data->threadinterface
+
extern "C"
{
int __pthread_attr_init (pthread_attr_t * attr);
