From 4ed54ca07b8115bd9e7813a1484d4c7936a25e70 Mon Sep 17 00:00:00 2001 From: rpj Date: Wed, 14 Oct 1998 03:06:39 +0000 Subject: Mon Oct 12 00:00:44 1998 Ross Johnson * implement.h (_pthread_tsd_key_table): New. * create.c (_pthread_start_call): Initialise per-thread TSD keys to NULL. * misc.c (pthread_once): Correct typo in comment. * implement.h (_pthread_destructor_push): Remove. (_pthread_destructor_pop): Remove. (_pthread_destructor_run_all): Rename from _pthread_destructor_pop_all. (_PTHREAD_TSD_KEY_DELETED): Add enum. (_PTHREAD_TSD_KEY_INUSE): Add enum. * cleanup.c (_pthread_destructor_push): Remove. (_pthread_destructor_pop): Remove. (_pthread_destructor_run_all): Totally revamped TSD. * dll.c (_pthread_TSD_keys_TlsIndex): Initialise. * tsd.c (pthread_setspecific): Totally revamped TSD. (pthread_getspecific): Ditto. (pthread_create): Ditto. (pthread_delete): Ditto. Sun Oct 11 22:44:55 1998 Ross Johnson * global.c (_pthread_tsd_key_table): Add new global. * implement.h (_pthread_tsd_key_t and struct _pthread_tsd_key): Add. (struct _pthread): Remove destructorstack. * cleanup.c (_pthread_destructor_run_all): Rename from _pthread_destructor_pop_all. The key destructor stack was made global rather than per-thread. No longer removes destructor nodes from the stack. Comments updated. --- ChangeLog | 39 +++++++++++++++++++++++ cleanup.c | 102 ++++++++---------------------------------------------------- create.c | 4 +++ dll.c | 13 ++++++++ global.c | 5 +++ implement.h | 24 +++++++++----- misc.c | 2 +- tsd.c | 86 ++++++++++++++++++++++++++++++++++++-------------- 8 files changed, 155 insertions(+), 120 deletions(-) diff --git a/ChangeLog b/ChangeLog index 51b81cf..3da9a14 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,42 @@ +Mon Oct 12 00:00:44 1998 Ross Johnson + + * implement.h (_pthread_tsd_key_table): New. + + * create.c (_pthread_start_call): Initialise per-thread TSD keys + to NULL. + + * misc.c (pthread_once): Correct typo in comment. + + * implement.h (_pthread_destructor_push): Remove. + (_pthread_destructor_pop): Remove. + (_pthread_destructor_run_all): Rename from _pthread_destructor_pop_all. + (_PTHREAD_TSD_KEY_DELETED): Add enum. + (_PTHREAD_TSD_KEY_INUSE): Add enum. + + * cleanup.c (_pthread_destructor_push): Remove. + (_pthread_destructor_pop): Remove. + (_pthread_destructor_run_all): Totally revamped TSD. + + * dll.c (_pthread_TSD_keys_TlsIndex): Initialise. + + * tsd.c (pthread_setspecific): Totally revamped TSD. + (pthread_getspecific): Ditto. + (pthread_create): Ditto. + (pthread_delete): Ditto. + +Sun Oct 11 22:44:55 1998 Ross Johnson + + * global.c (_pthread_tsd_key_table): Add new global. + + * implement.h (_pthread_tsd_key_t and struct _pthread_tsd_key): + Add. + (struct _pthread): Remove destructorstack. + + * cleanup.c (_pthread_destructor_run_all): Rename from + _pthread_destructor_pop_all. The key destructor stack was made + global rather than per-thread. No longer removes destructor nodes + from the stack. Comments updated. + 1998-10-06 Ben Elliston * condvar.c (cond_wait): Use POSIX, not Win32 mutex calls. diff --git a/cleanup.c b/cleanup.c index 7a7ea4e..c956ce7 100644 --- a/cleanup.c +++ b/cleanup.c @@ -120,112 +120,36 @@ _pthread_handler_pop_all(int stack, int execute) } } - -int -_pthread_destructor_push(void (* routine)(void *), pthread_key_t key) -{ - return _pthread_handler_push(_PTHREAD_DESTRUCTOR_STACK, - _PTHREAD_HANDLER_POP_LIFO, - routine, - (void *) key); -} - - -/* Remove all of the destructors associated with the key. */ -void -_pthread_destructor_pop(pthread_key_t key) -{ - _pthread_handler_node_t ** head; - _pthread_handler_node_t * current; - _pthread_handler_node_t * next; - - head = _PTHREAD_STACK(_PTHREAD_DESTRUCTOR_STACK); - current = *head; - - while (current != NULL) - { - next = current->next; - - /* The destructors associated key is in current->arg. */ - if (current->arg == (void *) key) - { - if (current == *head) - { - *head = next; - } - free(current); - } - current = next; - } -} - - -/* Run destructors for all non-NULL key values. - - FIXME: Currently we only run the destructors on the calling - thread's key values. The way I interpret POSIX semantics is that, - for each key that the calling thread has a destructor for, we need - to look at the key values of every thread and run the destructor on - it if the key value is non-NULL. - - The question is: how do we access the key associated values which - are private to other threads? - +/* Run destructors for all non-NULL key values for the calling thread. */ void -_pthread_destructor_pop_all() +_pthread_destructor_run_all() { - _pthread_handler_node_t ** head; - _pthread_handler_node_t * current; - _pthread_handler_node_t * next; - void (* func)(void *); + _pthread_tsd_key_t * k; void * arg; int count; - head = _PTHREAD_STACK(_PTHREAD_DESTRUCTOR_STACK); + k = _pthread_tsd_key_table; /* Stop destructor execution at a finite time. POSIX allows us to ignore this if we like, even at the risk of an infinite loop. */ for (count = 0; count < PTHREAD_DESTRUCTOR_ITERATIONS; count++) { - /* Loop through all destructors for this thread. */ - while (current != NULL) + /* Loop through all keys. */ + for (key = 0; key < _POSIX_THREAD_KEYS_MAX; key++) { - func = current->routine; - - /* Get the key value using the key which is in current->arg. */ - arg = pthread_getspecific((int) current->arg); + if (k->in_use != 1) + continue; - next = current->next; + arg = pthread_getspecific(key); - /* If the key value is non-NULL run the destructor, otherwise - unlink it from the list. - */ - if (arg != NULL) - { - if (func != NULL) - { - (void) func(arg); - } - } - else + if (arg != NULL && k->destructor != NULL) { - if (current == *head) - { - *head = next; - } - free(current); + (void) (k->destructor)(arg); } - current = next; - } - } - /* Free the destructor list even if we still have non-NULL key values. */ - while (*head != NULL) - { - next = (*head)->next; - free(*head); - *head = next; + k++; + } } } diff --git a/create.c b/create.c index d313a8a..f477585 100644 --- a/create.c +++ b/create.c @@ -22,13 +22,17 @@ STDCALL _pthread_start_call(void * us_arg) this thread's private stack so we're safe to leave data in them until we leave. */ pthread_t us; + void * keys[PTHREAD_KEYS_MAX]; unsigned (*func)(void *); void * arg; unsigned ret; us = (pthread_t) us_arg; + memset(keys, 0, sizeof(keys)); + (void) TlsSetValue(_pthread_threadID_TlsIndex, (LPVOID) us); + (void) TlsSetValue(_pthread_TSD_keys_TlsIndex, (LPVOID) keys); /* FIXME: For now, if priority setting fails then at least ensure that our records reflect true reality. */ diff --git a/dll.c b/dll.c index 5900ed9..83edcce 100644 --- a/dll.c +++ b/dll.c @@ -22,6 +22,10 @@ /* Global index for TLS data. */ DWORD _pthread_threadID_TlsIndex; +/* Global index for thread TSD key array. */ +DWORD _pthread_TSD_keys_TlsIndex; + + BOOL WINAPI PthreadsEntryPoint(HINSTANCE dllHandle, DWORD reason, LPVOID situation) @@ -42,9 +46,18 @@ BOOL WINAPI PthreadsEntryPoint(HINSTANCE dllHandle, { return FALSE; } + + /* Set up per thread TSD key array pointer. */ + _pthread_TSD_keys_TlsIndex = TlsAlloc(); + + if (_pthread_TSD_keys_TlsIndex == 0xFFFFFFFF) + { + return FALSE; + } break; case DLL_PROCESS_DETACH: + (void) TlsFree(_pthread_TSD_keys_TlsIndex); (void) TlsFree(_pthread_threadID_TlsIndex); break; diff --git a/global.c b/global.c index 6d55581..9ce662f 100644 --- a/global.c +++ b/global.c @@ -57,3 +57,8 @@ pthread_t _pthread_win32handle_map[_PTHREAD_MAX_THREADS]; /* Per thread mutex locks. */ pthread_mutex_t _pthread_threads_mutex_table[_PTHREAD_MAX_THREADS]; +/* Global TSD key array. */ +_pthread_tsd_key_t _pthread_tsd_key_table[_POSIX_THREAD_KEYS_MAX]; + +/* Index to the next available TSD key. */ +int _pthread_tsd_key_next = 0; diff --git a/implement.h b/implement.h index bf55e91..a87edb2 100644 --- a/implement.h +++ b/implement.h @@ -19,6 +19,11 @@ enum { _PTHREAD_REUSE }; +enum { + _PTHREAD_TSD_KEY_DELETED, + _PTHREAD_TSD_KEY_INUSE +}; + #define _PTHREAD_VALID(T) \ ((T) != NULL \ && ((T)->ptstatus == _PTHREAD_NEW \ @@ -40,6 +45,14 @@ struct _pthread_handler_node { void * arg; }; +/* TSD key element. */ +typedef struct _pthread_tsd_key _pthread_tsd_key_t; + +struct _pthread_tsd_key { + int in_use; + void (* destructor)(void *); +}; + /* Stores a thread call routine and argument. */ typedef struct { unsigned (*routine)(void *); @@ -84,7 +97,6 @@ struct _pthread { /* These must be kept in this order and together. */ _pthread_handler_node_t * cleanupstack; - _pthread_handler_node_t * destructorstack; _pthread_handler_node_t * forkpreparestack; _pthread_handler_node_t * forkparentstack; _pthread_handler_node_t * forkchildstack; @@ -107,12 +119,7 @@ void _pthread_handler_pop(int stack, void _pthread_handler_pop_all(int stack, int execute); -int _pthread_destructor_push(void (*routine)(void *), - pthread_key_t key); - -void _pthread_destructor_pop(pthread_key_t key); - -void _pthread_destructor_pop_all(); +void _pthread_destructor_run_all(); /* Primitives to manage threads table entries. */ @@ -161,6 +168,9 @@ extern pthread_t _pthread_win32handle_map[]; /* Per thread mutex locks. */ extern pthread_mutex_t _pthread_threads_mutex_table[]; +/* Global TSD key array. */ +extern _pthread_tsd_key_t _pthread_tsd_key_table[]; + #endif /* _IMPLEMENT_H */ diff --git a/misc.c b/misc.c index d39af50..659a4eb 100644 --- a/misc.c +++ b/misc.c @@ -13,7 +13,7 @@ int pthread_once(pthread_once_t *once_control, void (*init_routine)(void)) { - /* A flag, allocated per invocation, that indicates if the amotic + /* A flag, allocated per invocation, that indicates if the atomic test-and-set occured. */ unsigned short flag = 0; diff --git a/tsd.c b/tsd.c index 015af85..8e53b37 100644 --- a/tsd.c +++ b/tsd.c @@ -12,14 +12,40 @@ * In a word: Destructors * * POSIX 1003.1 1996, Section 17 allows for optional destructor functions - * to be associated with each key value. The destructors are called from - * the creating thread, which means that the calling thread must have access - * to the TSD keys of all active threads. + * to be associated with each key value. * - * If we use Win32 TLS then this is not possible since Tls*Value() - * functions don't allow us to access other than our own [thread's] key. + * This is my (revised) understanding of how destructors work: * - * As a result, these routines need to be redesigned. + * A key is created by a single thread, which then provides in every + * existing thread a TSD matching the same key, but initialised + * to NULL. Each new thread will also get a matching key with value NULL. + * The creating thread can optionally associate a function, called a + * destructor, with the key. + * + * When each thread exits, it calls the destructor function, which + * will then perform an action on that threads key value + * only. (Previously I thought that only the key creating thread ran + * the destructor on the key in all threads. That proposition is + * sounding scarier by the minute.) + * + * SOME APPROACHES TO MANAGING TSD MEMORY + * + * We could simply allocate enough memory on process startup to hold + * all possible data for all possible threads. + * + * We could allocate memory for just a table to hold a single pointer + * for each of POSIX_THREAD_KEYS_MAX keys. pthread_key_create() could then + * allocate space for POSIX_THREADS_MAX key values in one hit and store + * the location of the array in the first table. + * + * The standard also suggests that each thread might store key/value pairs + * on its private stack. This seems like a good idea. I had concerns about + * memory leaks and key re-use if a key was deleted, but the standard talks + * at length on this and basically says it's up to the application to + * make sure everything goes smoothly here, making sure that proper cleanup + * is done before a key is deleted. (section B.17.1.3 in particular) + * + * One more thing to note: destructors must never be called on deleted keys. */ #include @@ -30,21 +56,17 @@ int pthread_key_create(pthread_key_t *key, void (*destructor)(void *)) { - DWORD index; + pthread_key_t k; + + if (_pthread_tsd_key_next >= PTHREAD_KEYS_MAX) + return EAGAIN; - index = TlsAlloc(); - if (index == 0xFFFFFFFF) - { - return EAGAIN; - } + k = _pthread_tsd_key_next++; - /* Only modify the `key' parameter if allocation was successful. */ - *key = index; + _pthread_tsd_key_table[k].in_use = _PTHREAD_TSD_KEY_INUSE; + _pthread_tsd_key_table[k].destructor = destructor; - if (destructor != NULL) - { - return (_pthread_destructor_push(destructor, *key)); - } + *key = k; return 0; } @@ -52,20 +74,38 @@ pthread_key_create(pthread_key_t *key, void (*destructor)(void *)) int pthread_setspecific(pthread_key_t key, void *value) { - return (TlsSetValue(key, value) == FALSE) ? EINVAL : 0; + void ** keys; + + if (_pthread_tsd_key_table[key].in_use != _PTHREAD_TSD_KEY_INUSE) + return EINVAL; + + keys = (void **) TlsGetValue(_pthread_TSD_keys_TlsIndex); + keys[key] = value; + + return 0; } void * pthread_getspecific(pthread_key_t key) { - return TlsGetValue(key); + void ** keys; + + if (_pthread_tsd_key_table[key].in_use != _PTHREAD_TSD_KEY_INUSE) + return EINVAL; + + keys = (void **) TlsGetValue(_pthread_TSD_keys_TlsIndex); + return keys[key]; } int pthread_key_delete(pthread_key_t key) { - /* Remove this key's destructors. */ - _pthread_destructor_pop(key); + if (_pthread_tsd_key_table[key].in_use != _PTHREAD_TSD_KEY_INUSE) + return EINVAL; - return (TlsFree(key) == FALSE) ? EINVAL : 0; + _pthread_tsd_key_table[key].in_use = _PTHREAD_TSD_KEY_DELETED; + _pthread_tsd_key_table[key].destructor = NULL; + + return 0; } + -- cgit v1.2.3