Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to PEP-539 draft #248

Merged
merged 9 commits into from Apr 30, 2017
115 changes: 96 additions & 19 deletions pep-0539.txt
Expand Up @@ -24,43 +24,106 @@ included in ``Python.h`` either directly or indirectly), this proposal probably
only affects CPython, but might also affect other interpreter implementations
(PyPy?) that implement parts of the CPython API.

This is motivated primarily by the fact that the old API uses ``int`` to
represent TLS keys across all platforms, which is neither POSIX-compliant,
nor portable in any practical sense [1]_.

.. note::

Throughout this document the acronym "TLS" refers to Thread Local
Storage and should not be confused with "Transportation Layer Security"
protocols.


Specification
=============

The current API for TLS used inside the CPython interpreter consists of 5
The current API for TLS used inside the CPython interpreter consists of 6
functions::

PyAPI_FUNC(int) PyThread_create_key(void)
PyAPI_FUNC(void) PyThread_delete_key(int key)
PyAPI_FUNC(int) PyThread_set_key_value(int key, void *value)
PyAPI_FUNC(void *) PyThread_get_key_value(int key)
PyAPI_FUNC(void) PyThread_delete_key_value(int key)
PyAPI_FUNC(void) PyThread_ReInitTLS(void)

These would be superseded by a new set of analogous functions::

PyAPI_FUNC(int) PyThread_tss_create(Py_tss_t *key)
PyAPI_FUNC(void) PyThread_tss_delete(Py_tss_t key)
PyAPI_FUNC(void) PyThread_tss_delete(Py_tss_t *key)
PyAPI_FUNC(int) PyThread_tss_set(Py_tss_t key, void *value)
PyAPI_FUNC(void *) PyThread_tss_get(Py_tss_t key)
PyAPI_FUNC(void) PyThread_tss_delete_value(Py_tss_t key)
PyAPI_FUNC(void) PyThread_ReInitTSS(void)

The specification also adds three new features:

* A new type ``Py_tss_t``--an opaque type the definition of which may
depend on the underlying TLS implementation. It is defined::

typedef struct {
bool _is_initialized;
NATIVE_TLS_KEY_T _key;
} Py_tss_t;

where ``NATIVE_TLS_KEY_T`` is a macro whose value depends on the
underlying native TLS implementation (e.g. ``pthread_key_t``).

* A constant default value for ``Py_tss_t`` variables,
``Py_tss_NEEDS_INIT``.

* A new inline function::

static inline bool PyThread_tss_is_created(Py_tss_t key)

along with a new type ``Py_tss_t``--an opaque type the definition of which
is undefined here, and may depend on the underlying TLS implementation.
which returns ``true`` if the given ``Py_tss_t`` has been initialized
(i.e. by ``PyThread_tss_create``).

The new ``PyThread_tss_`` functions are almost exactly analogous to their
original counterparts with a minor difference: Whereas
``PyThread_create_key`` takes no arguments and returns a TLS key as an
``int``, ``PyThread_tss_create`` takes a ``Py_tss_t*`` as an argument, and
returns a ``Py_tss_t`` by pointer--the ``int`` return value is a status,
returning zero on success and non-zero on failure. The meanings of non-zero
status codes are not not defined by this specification.
``int``, ``PyThread_tss_create`` takes a ``Py_tss_t*`` as an argument and
returns an ``int`` status code. The behavior of ``PyThread_tss_create`` is
undefined if the value pointed to by the ``key`` argument is not initialized
by ``Py_tss_NEEDS_INIT``. The returned status status code is zero on success
and non-zero on failure. The meanings of non-zero status codes are not
otherwise defined by this specification.

The old ``PyThread_*_key*`` functions will be marked as deprecated.
Similarly ``PyThread_tss_delete`` is passed a ``Py_tss_t*`` whereas
previouly the key was passed to ``PyThread_delete_key`` by value.

The old ``PyThread_*_key*`` functions will be marked as deprecated in the
documentation, but will not generate runtime deprecation warnings.

Additionally, on platforms where ``sizeof(pthread_key_t) != sizeof(int)``,
``PyThread_create_key`` will return immediately with a failure status, and
the other TLS functions will all be no-ops.
the other TLS functions will all be no-ops on such platforms.

Example
-------

With the proposed changes, a TSS key is initialized like::

static Py_tss_t tss_key = Py_tss_NEEDS_INIT;
if (PyThread_tss_create(&tss_key)) {
/* ... handle key creation failure ... */
}

The initialization state of the key can then be checked like::

assert(PyThread_tss_is_created(tss_key));

The rest of the API is used analogously to the old API::

int the_value = 1;
if (PyThread_tss_get(tss_key) == NULL) {
PyThread_tss_set(tss_key, (void *)&the_value);
assert(PyThread_tss_get(tss_key) != NULL);
}
/* ... once done with the key ... */
PyThread_tss_delete(&tss_key);
assert(!PyThread_tss_is_created(tss_key));


Motivation
Expand Down Expand Up @@ -97,7 +160,7 @@ However, as issue #25658 points out, there are at least some platforms
modern and POSIX-compliant pthreads implementations, but are not compatible
with Python's API because their ``pthread_key_t`` is defined in a way that
cannot be safely cast to ``int``. In fact, the possibility of running into
this problem was raised by MvL at the time pthreads TLS was added [1]_.
this problem was raised by MvL at the time pthreads TLS was added [2]_.

It could be argued that PEP-11 makes specific requirements for supporting a
new, not otherwise officially-support platform (such as CloudABI), and that
Expand Down Expand Up @@ -127,6 +190,18 @@ part of the C11 threads API. However, this is in no way meant to imply
compatibility with or support for the C11 threads API, or signal any future
intention of supporting C11--it's just the influence for the naming and design.

The inclusion of the special default value ``Py_tss_NEEDS_INIT`` is required
by the fact that not all native TLS implementations define a sentinel value
for uninitialized TLS keys. For example, on Windows a TLS key is
represented by a ``DWORD`` (``unsigned int``) and its value must be treated
as opaque [3]_. So there is no unsigned integer value that can be safely
used to represent an uninititalized TLS key on Windows. Likewise, POSIX
does not specify a sentintel for an uninitialized ``pthread_key_t``, instead
relying on the ``pthread_once`` interface to ensure that a given TLS key is
initialized only once per-process. Therefore, the ``Py_tss_t`` type
contains an explicit ``._is_initialized`` that can indicate the key's
initialization state independent of the underlying implementation.

Changing ``PyThread_create_key`` to immediately return a failure status on
systems using pthreads where ``sizeof(int) != sizeof(pthread_key_t)`` is
intended as a sanity check: Currently, ``PyThread_create_key`` may report
Expand Down Expand Up @@ -159,15 +234,15 @@ Rejected Ideas

* Affected platforms should not define ``Py_HAVE_NATIVE_TLS``: This is a more
acceptable alternative to the previous idea, and in fact there is a patch to
do just that [2]_. However, CPython's internal TLS implementation being
do just that [4]_. However, CPython's internal TLS implementation being
"slower and clunkier" in general than native implementations still needlessly
hobbles performance on affected platforms. At least one other module
(``tracemalloc``) is also broken if Python is built without
``Py_HAVE_NATIVE_TLS``.

* Keep the existing API, but work around the issue by providing a mapping from
``pthread_key_t`` values to ``int`` values. A couple attempts were made at
this ([3]_, [4]_), but this only injects needless complexity and overhead
this ([5]_, [6]_), but this only injects needless complexity and overhead
into performance-critical code on platforms that are not currently affected
by this issue (such as Linux). Even if use of this workaround were made
conditional on platform compatibility, it introduces platform-specific code
Expand All @@ -178,7 +253,7 @@ Rejected Ideas
Implementation
==============

An initial version of a patch [5]_ is available on the bug tracker for this
An initial version of a patch [7]_ is available on the bug tracker for this
issue.


Expand All @@ -191,8 +266,10 @@ This document has been placed in the public domain.
References and Footnotes
========================

.. [1] https://bugs.python.org/msg116292
.. [2] http://bugs.python.org/file45548/configure-pthread_key_t.patch
.. [3] http://bugs.python.org/file44269/issue25658-1.patch
.. [4] http://bugs.python.org/file44303/key-constant-time.diff
.. [5] http://bugs.python.org/file45763/pythread-tss.patch
.. [1] http://bugs.python.org/issue25658
.. [2] https://bugs.python.org/msg116292
.. [3] https://msdn.microsoft.com/en-us/library/windows/desktop/ms686801(v=vs.85).aspx
.. [4] http://bugs.python.org/file45548/configure-pthread_key_t.patch
.. [5] http://bugs.python.org/file44269/issue25658-1.patch
.. [6] http://bugs.python.org/file44303/key-constant-time.diff
.. [7] http://bugs.python.org/file46379/pythread-tss-3.patch