Web lists-archives.com

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






On 5/21/2019 5:27 PM, Jeff King wrote:
On Tue, May 21, 2019 at 12:33:58PM -0700, Jeff Hostetler via GitGitGadget wrote:

As Duy suggested, pthread_getspecific() just returns NULL when NO_PTHREADS
is defined. And pthread_setspecific() silently does not nothing. So this
problem was hidden from view.

I have to wonder if we should update pthread_*specific() to call BUG() when
NO_PTHREADS is defined as a way to catch unguarded usages easier or make
this issue more clear.

I think it should actually store the data asked for by the caller, as if
we were the single thread running. We discussed this as the time of
refactoring NO_PTHREADS, but there was only one caller that would have
benefited. Now there are two. ;)

Discussion in the subthread of this patch:

   https://public-inbox.org/git/20181027071003.1347-2-pclouds@xxxxxxxxx/

-Peff


I was wondering about that too as the proper long term solution.
We would need (as the discussion suggests [1]) to properly
respect/represent the pthread_key_t argument.

For now, I've guarded my usage of pthread_getspecific() in the trace2
(similar to what index-pack does), so its not urgent that we update it.
And I'd rather we take this simple trace2 fix now and not try to combine
it with fixes for the pthread macros.  Especially now as we're in the RC
cycle for 2.22.

I'll make a note to revisit the pthread code after 2.22.

Thanks
Jeff


[1] https://public-inbox.org/git/CACsJy8DLW_smOJd6aCoRcJZxQ2Lzut5US=sPadj7=fhne0UHGg@xxxxxxxxxxxxxx/