Re: [PATCH 0/1] trace2: fix tracing when NO_PTHREADS is defined

From: Jeff King <peff@peff.net>
Date: May 28, 2019, 06:28 UTC
Message-ID: <20190528062804.GE7946@sigill.intra.peff.net>
In-Reply-To: <CACsJy8D7w4sC_tchx-Q80PCiu+2hYfkZo22_Vb3vgJ+xvgUAAA@mail.gmail.com>

On Sat, May 25, 2019 at 05:43:55PM +0700, Duy Nguyen wrote:

Show 8 quoted lines

> > +typedef struct {
> > +       void *data;
> > +       /* extra indirection because setspecific is passed key by value */
> > +       void **vdata;
> 
> Ha! I was thinking a separate key->value mapping which is complicated
> in C. But this works pretty well for a single thread, and it even
> supports multiple keys.

I really wish that all of the functions passed the pthread_key_t by reference. That would make it possible to define the key as a single pointer.

I'm not sure if pthread_key_t's are meant to be shallow-copyable. I.e., should this work:

  void foo(pthread_key_t *out)
  {
	pthread_key_t tmp;
	pthread_key_create(&tmp, NULL);
	*out = tmp;
  }
  ...
  pthread_key_t k;
  foo(&k);
  pthread_setspecific(k, some_ptr);

It does not with my proposed plan, because the pointer in tmp.data went out of scope, leaving tmp.vdata (and thus k.vdata) as a dangling pointer.

The code above seems like a vaguely crazy thing to do. But if we want to be absolutely paranoid, we'd have to malloc() an extra pointer in the create() function, instead of carrying it inside the key. Or just make a global "void *thread_specific_data[PTHREAD_KEYS_MAX]" and make each key an integer index into it.

It's pretty clear that they expect one of those two implementations, given that POSIX says key creation can report either ENOMEM, or EAGAIN if we exceed PTHREAD_KEYS_MAX. :)

-Peff