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@gmail.com>:
On Fri, Dec 30, 2016 at 5:05 PM, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 29 December 2016 at 22:12, Erik Bray <erik.m.bray@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