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

[Erik Bray]

On Sat, Dec 17, 2016 at 8:21 AM, Stephen J. Turnbull
<turnbull.stephen.fw at u.tsukuba.ac.jp> wrote:
> Erik Bray writes:
>
>  > Abstract
>  > ========
>  >
>  > The proposal is to add a new Thread Local Storage (TLS) API to CPython
>  > which would supersede use of the existing TLS API within the CPython
>  > interpreter, while deprecating the existing API.
>
> Thank you for the analysis!

And thank *you* for the feedback!

> Question:
>
>  > Further, the old PyThread_*_key* functions will be marked as
>  > deprecated.
>
> Of course, but:
>
>  > Additionally, the pthread implementations of the old
>  > PyThread_*_key* functions will either fail or be no-ops on
>  > platforms where sizeof(pythead_t) != sizeof(int).
>
> Typo "pythead_t" in last line.

Thanks, yes, that was suppose to be pthread_key_t of course.  I think
I had a few other typos too.

> I don't understand this.  I assume that there are no such platforms
> supported at present.  I would think that when such a platform becomes
> supported, code supporting "key" functions becomes unsupportable
> without #ifdefs on that platform, at least directly.  So you should
> either (1) raise UnimplementedError, or (2) provide the API as a
> wrapper over the new API by making the integer keys indexes into a
> table of TSS'es, or some such device.  I don't understand how (3)
> "make it a no-op" can be implemented for PyThread_create_key -- return
> 0 or -1?  That would only work if there's a failure return status like
> 0 or -1, and it seems really dangerous to me since in general a lot of
> code doesn't check status even though it should.  Even for code
> checking the status, the error message will be suboptimal ("creation
> failed" vs. "unimplemented").

Masayuki already explained this downthread I think, but I could have
probably made that section more precise.  The point was that
PyThread_create_key should immediately return -1 in this case.  This
is just a subtle difference over the current situation, which is that
PyThread_create_key succeeds, but the key is corrupted by being cast
to an int, so that later calls to PyThread_set_key_value and the like
fail unexpectedly.  The point is that PyThread_create_key (and we're
only talking about the pthread implementation thereof, to be clear)
must fail immediately if it can't work correctly.

#ifdefs on the platform would not be necessary--instead, Masayuki's
patch adds a feature check in configure.ac for sizeof(int) ==
sizeof(pthread_key_t).  It should be noted that even this check is not
100% perfect, as on Linux pthread_key_t is an unsigned int, and so
technically can cause Python's signed int key to overflow, but there's
already an explicit check for that (which would be kept), and it's
also a very unlikely scenario.

> I gather from references to casting pthread_key_t to unsigned int and
> back that there's probably code that does this in ways making (2) too
> dangerous to support.  If true, perhaps that should be mentioned here.

It's not necessarily too dangerous, so much as not worth the trouble,
IMO.  Simpler to just provide, and immediately use the new API and
make the old one deprecated and explicitly not supported on those
platforms where it can't work.

Thanks,
Erik