On 31 December 2016 at 08:24, Masayuki YAMAMOTO <ma3yuki.8mamo10@gmail.com> wrote:

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.

That's not what we currently do for the EnsureGIL autoTLS key and the tracemalloc key though - the reentrant key creation is part of "create-if-needed" flows where the key creation is silently skipped if the key already exists.

Changing that would require some further research into how we ended up with the current approach, while carrying it over into the new API design would be the default option.

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.

I agree it would be good to add more test cases for this scenario to the test suite.

Cheers,

Nick.

Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia