Making sense of TSD

[Mike Hommey]

On Wed, Apr 11, 2012 at 09:22:47PM +0200, Mike Hommey wrote:
> On Wed, Apr 11, 2012 at 12:15:37PM -0700, Jason Evans wrote:
> > 
> > On Apr 11, 2012, at 12:08 PM, Mike Hommey wrote:
> > 
> > > On Wed, Apr 11, 2012 at 12:05:05PM -0700, Jason Evans wrote:
> > >> On Apr 11, 2012, at 10:22 AM, Mike Hommey wrote:
> > >>> I'm trying to make sense of the tsd implementation without
> > >>> __thread, which would seem to fit the tls model on windows. But I
> > >>> don't see why the wrapper struct is needed at all. The first thing
> > >>> that bothers me is that the isstatic == true case is set to abort
> > >>> on opt_abort, yet is supported. And it doesn't seem very obvious
> > >>> why initialized is needed.
> > >> 
> > >> The wrapper struct is needed with pthreads TSD because
> > >> pthread_[gs]etspecific() only operate on (void *) pointers.  Thus
> > >> it's impossible to store more than a pointer-sized item with
> > >> pthreads TSD unless that pointer refers to an allocated structure.
> > >> I don't remember the details of Windows's TLS/TSD API; if it can
> > >> store any type/structure, then no wrapper struct is needed.
> > > 
> > > Well, the void* pointer could just point to whatever structure you
> > > want to store, instead of having a struct containing two bools which
> > > usefulness I'm doubtful about, and a pointer to that allocated
> > > structure.  At least it seems so. That was my question.
> > 
> > The utility of isstatic could be argued, but the initialized flag is
> > critical to how cleanup handlers work.  Yes, these two flags could be
> > separate TSD keys (with all the requisite bool<-->(void *) casting),
> > but the actual data being stored has to be allocated anyway, so this
> > is IMO a cleaner solution.  If Windows supports arbitrary types in a
> > similar fashion to __thread, then the wrapper structure certainly
> > isn't needed, since the flags can be separate keys.
> 
> I must be missing something, because I still don't see how initialized
> is any useful. When a thread is created, the specific data value
> associated with the key is null and the destructor/cleanup is not
> called. Until a value is actually set with setspecific.
> 
> Also, come to think of it, I don't even see how that static business
> works: while the wrapper data is allocated on heap, the actual type data
> is allocated statically, and that will be shared across threads. So in
> practice, this means all threads are going to share the same data, and
> cross-overwrite it.

Reading preprocessed code makes it clearer: the "static" is mostly useless
in that branch, and can be removed without any change in functionality.
Doing so also saves some bss space.

I also tested modifying the code as follows (getting completely rid of the
wrappers), and it seems to work (although it's missing failure paths):

a_name##_tsd_boot(void)							\
{									\
	if (pthread_key_create(&a_name##_tsd,				\
	    a_cleanup) != 0)						\
		return (true);						\
	a_name##_booted = true;						\
	return (false);							\
}									\
									\
a_attr a_type *								\
a_name##_tsd_get(void)							\
{									\
	a_type *value = (a_type *) pthread_getspecific(a_name##_tsd);	\
	if (value == NULL) {						\
		a_type tsd_init_data = a_initializer;			\
		value = (a_type *) malloc_tsd_malloc(sizeof(a_type));	\
		*value = tsd_init_data;					\
		pthread_setspecific(a_name##_tsd, (void *)value);	\
	}								\
	return value;							\
}									\
a_attr void								\
a_name##_tsd_set(a_type *val)						\
{									\
	a_type *value = a_name##_tsd_get();				\
	*value = *(val);						\
}

FWIW, Win32's TlsAlloc and TlsGetValue/TlsSetValue basically work as
respectively pthread_key_create, pthread_getspecific and
pthread_setspecific, except there is no destructor function.
function. This means we need some way to enumerate the tsd values and
cleanup functions to run them. Any preference on how to do that? I'm
thinking about using a separate section to store pointers to the tsd
variables, so that the linker would do the work, instead of having to
create a list somewhere in the source. Another possibility I can see
is to group all tsd definitions.

Cheers,

Mike