[Python-ideas] New PyThread_tss_ C-API for CPython

[Masayuki YAMAMOTO]

I have read the discussion and I'm sure that use structure as Py_tss_t
instead of platform-specific data type. Just as Steve said that Py_tss_t
should be genuinely treated as an opaque type, the key state checking
should provide macros or inline functions with name like
PyThread_tss_is_created. Well, I'd resolve the specification a bit more :)

If PyThread_tss_create is called with the created key, it is no-op but
which the function should succeed or fail? In my opinion, It is better to
return a failure because it is a high possibility that the code is
incorrect for multiple callings of PyThread_tss_create for One key.

In this opinion PyThread_tss_is_created should return a value as follows:
(A) False while from after defining with Py_tss_NEED_INIT to before calling
PyThread_tss_create
(B) True after calling PyThread_tss_create succeeded
(C) Unchanging before and after calling PyThread_tss_create failed
(D) False after calling PyThread_tss_delete regardless of timing
(E) For other functions, the return value of PyThread_tss_is_created does
not change before and after calling

I think that it is better to write a test about the state of the Py_tss_t.

Kind regards,
Masayuki

2016-12-31 2:38 GMT+09:00 Erik Bray <erik.m.bray at gmail.com>:

> On Fri, Dec 30, 2016 at 5:05 PM, Nick Coghlan <ncoghlan at gmail.com> wrote:
> > On 29 December 2016 at 22:12, Erik Bray <erik.m.bray at gmail.com> wrote:
> >>
> >> 1) CPython's TLS: Defines -1 as an uninitialized key (by fact of the
> >> implementation--that the keys are integers starting from zero)
> >> 2) pthreads: Does not definite an uninitialized default value for
> >> keys, for reasons described at [1] under "Non-Idempotent Data Key
> >> Creation".  I understand their reasoning, though I can't claim to know
> >> specifically what they mean when they say that some implementations
> >> would require the mutual-exclusion to be performed on
> >> pthread_getspecific() as well.  I don't know that it applies here.
> >
> >
> > That section is a little weird, as they describe two requests (one for a
> > known-NULL default value, the other for implicit synchronisation of key
> > creation to prevent race conditions), and only provide the justification
> for
> > rejecting one of them (the second one).
>
> Right, that is confusing to me as well. I'm guessing the reason for
> rejecting the first is in part a way to force us to recognize the
> second issue.
>
> > If I've understood correctly, the situation they're worried about there
> is
> > that pthread_key_create() has to be called at least once-per-process, but
> > must be called before *any* call to pthread_getspecific or
> > pthread_setspecific for a given key. If you do "implicit init" rather
> than
> > requiring the use of an explicit mechanism like pthread_once (or our own
> > Py_Initialize and module import locks), then you may take a small
> > performance hit as either *every* thread then has to call
> > pthread_key_create() to ensure the key exists before using it, or else
> > pthread_getspecific() and pthread_setspecific() have to become
> potentially
> > blocking calls. Neither of those is desirable, so it makes sense to leave
> > that part of the problem to the API client.
> >
> > In our case, we don't want the implicit synchronisation, we just want the
> > known-NULL default value so the "Is it already set?" check can be moved
> > inside the library function.
>
> Okay, we're on the same page here then.  I just wanted to make sure
> there wasn't anything else I was missing in Python's case.
>
> >> 3) windows: The return value of TlsAlloc() is a DWORD (unsigned int)
> >> and [2] states that its value should be opaque.
> >>
> >> So in principle we can cover all cases with an opaque struct that
> >> contains, as its first member, an is_initialized flag.  The tricky
> >> part is how to initialize the rest of the struct (containing the
> >> underlying implementation-specific key).  For 1) and 3) it doesn't
> >> matter--it can just be zero.  For 2) it's trickier because there's no
> >> defined constant value to initialize a pthread_key_t to.
> >>
> >> Per Nick's suggestion this can be worked around by relying on C99's
> >> initialization semantics. Per [3] section 6.7.8, clause 21:
> >>
> >> """
> >> If there are fewer initializers in a brace-enclosed list than there
> >> are elements or members of an aggregate, or fewer characters in a
> >> string literal used to initialize an array of known size than there
> >> are elements in the array, the remainder of the aggregate shall be
> >> initialized implicitly the same as objects that have static storage
> >> duration.
> >> """
> >>
> >> How objects with static storage are initialized is described in the
> >> previous page under clause 10, but in practice it boils down to what
> >> you would expect: Everything is initialized to zero, including nested
> >> structs and arrays.
> >>
> >> So as long as we can use this feature of C99 then I think that's the
> >> best approach.
> >
> >
> >
> > I checked PEP 7 to see exactly which features we've added to the
> approved C
> > dialect, and designated initialisers are already on the list:
> > https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> >
> > So I believe that would allow the initializer to be declared as something
> > like:
> >
> >     #define Py_tss_NEEDS_INIT {.is_initialized = false}
>
> Great!  One could argue about whether or not the designated
> initializer syntax also incorporates omitted fields, but it would seem
> strange to insist that it doesn't.
>
> Have a happy new year,
>
> Erik
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-ideas/attachments/20161231/2a6c245c/attachment-0001.html>