[issue25658] PyThread assumes pthread_key_t is an integer, which is against POSIX

[Erik Bray]

Erik Bray added the comment:

Hi,

First of all, I should be clear this is not just about CloudABI.  In fact I don't even know what CloudABI is.  I'm personally working toward getting a working/stable buildbot for Cygwin, so that I can move Python toward supporting it again.  Cygwin has not been officially unsupported, but support for it is broken due to lack of a buildbot and lack of a maintainer, both of which I'm willing to offer.  I can't get a stable buildbot though until some issues with the build are resolved, and this is a major blocker.  If you prefer I can also work on a more formal plan to clarify Python's support for Cygwin which is currently in limbo.

Not saying a solution to this needs to be merged ASAP as I can work on a branch, but in the meantime it's nice to have an initial fix for the issue so that it can be worked around--I'm sure other platforms that have this issue, such as the CloudABI folks, will appreciate that as well.

Second of all, the point must be made that I'm not suggesting this be changed just in order to support some specific platform.  The fact remains that the existing code is misusing the pthread API and is fragile to begin with.  "It works on Linux" is not an excuse--not only does it suggest a bias, but the fact that it even works on Linux is an accident of implementation. The implementation of the pthread_key_t type could change tomorrow, and it would be Python's fault if that breaks things (not that I think that's at all likely to happen, though I'd be less surprised if OSX suddenly changed :)

As for the performance I agree that PyThread_(get|set)_key_value should be fast.  This may be O(n) but how big, typically, is n?  In a normal run of the Python interpreter n=1, and the autoTLSkey is always the first in the list.  It is only larger if some third-party code is using the PyThread_ API for TLS (and in most typical uses this will only be a few keys at the most, though I couldn't even find any examples in the wild where third-party code is using these functions).

n could also grow on forks, or manual Py_Finialize/Py_Initialize.  This patch didn't implement PyThread_ReInitTLS but it could, in which case it would reset next_key_id to 0 for each child process.  Likewise, PyThread_ReInitTLS could also be called from Py_Initialize, which should be harmless.

So TL;DR I'm not really convinced by the performance argument.  But even if that were an issue other implementations are possible; this was just among the simplest and least wasteful.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue25658>
_______________________________________