Re: [basic-improvements] Remove __gthread_key_dtor

[Zack Weinberg <zack at codesourcery dot com>]

On Thu, Oct 31, 2002 at 02:31:21PM -0800, Eric Christopher wrote:
> 
> re mutexes: Sony only has mutexes and not a real thread system at all.
> So, we get to "invent" things via locking and mutexes.

Understood.  In fact, looking over your code, I just wrote some very
similar stuff for VxWorks (which has threads, but neither once_t nor
thread-specific data as POSIX defines it).

> Now, I may have a problem with my implementation based on rth's email,
> but here's the dtor function as it currently exists:
> 
> static inline int
> __gthread_key_dtor (__gthread_key_t key, void *ptr)
> {
>   /* Just reset the key value to zero.  */
>   if (ptr)
>     return __gthread_setspecific (key, 0);
>   else
>     return 0;
> }

This implementation is identical with several of the ones I calmly
wiped out of existence as "does nothing useful".  The thread
destruction logic is (should be) responsible for doing this.

I see a number of problems with your fake TSD implementation, but
nothing that requires you to have a __gthread_key_dtor routine.  See
below.

> static void *key_value[THR_ID_NUM][KEY_NUM] = { { 0 }, { 0 } };
> static int   key_use[KEY_NUM] = { 0 };
> 
> /* Mutex for locking key bitmap.  */
> static __gthread_mutex_t key_mutex;

There needs to be just one version of these variables, but gthr.h is
included all over the place - this needs to go out-of-line, into a
libgcc.a extra part, probably along with the code that uses it.

I take it there can be only 256 threads?

> static inline int
> __gthread_once (__gthread_once_t *once, void (*func) ())
> {
>   return 0;
> }

You're going to want a real version of this function.  It's used in
the unwinder, but also...

> static inline int
> __gthread_key_create (__gthread_key_t *keyp, void (*dtor) (void *))
> {
>   int i;
> 
>   __gthread_mutex_lock (&key_mutex);
>   for (i = 0; i < KEY_NUM; i++)
>     {
>       if (!key_use[i] && dtor != 0)
>         {
>           key_use[i] = 1;
>           __gthread_mutex_unlock (&key_mutex);
>           *keyp = i;
>           return 0;
>         }
>     }
>   __gthread_mutex_unlock (&key_mutex);
>   return -1;
> }

where does key_mutex get initialized?  I have the same requirement on
mutexes in VxWorks (must use an initialization function) and wound up
using __gthread_once to call __gthread_mutex_init_function on the
mutex used to protect the keys table.

There is no requirement that dtor be nonzero, and you threw away its
value; this will cause memory leaks.  You're going to need to find
some way to get a callback run at thread exit time.  And that callback
can do the clear-out-the-value thing.

> static inline int
> __gthread_setspecific (__gthread_key_t key, const void *ptr)
> {
>   if (key > KEY_NUM || !key_use[key])
>     return -1;
> 
>   key_value[GetThreadId ()][key] = (void *)ptr;
>   return 0;
> }

This races with key_delete.  You'll have to have it take the lock, too.

zw