On Sat, Dec 17, 2016 at 8:21 AM, Stephen J. Turnbull <turnbull.stephen.fw@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