Updates to PEP-539 draft (#248)

* Typo: "not not" (thanks Florent Hivert)
* Added a terminology note that hopefully should clear up confusion for new-comers to the PEP, as there have also been recent PEPs/discussions surrounding TLS in the HTTPS sense
* Adds a brief summary of the motivation to the abstract, along with a link to the relevant bug
* Updated the new API specification to reflect additions/changes since the last draft
* Added a usage example to the specification
* Added an explanation of the need for Py_tss_NEEDS_INIT to the rationalization
* Other minor wording updates and typo fixes

pep-0539.txt

@@ -24,11 +24,21 @@ 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)
@@ -36,31 +46,84 @@ functions::
     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)
 
-along with a new type ``Py_tss_t``--an opaque type the definition of which
+The specification also adds three new features:
-is undefined here, and may depend on the underlying TLS implementation.
+
+* 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)
+
+  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
+``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,
+returns an ``int`` status code. The behavior of ``PyThread_tss_create`` is
-returning zero on success and non-zero on failure.  The meanings of non-zero
+undefined if the value pointed to by the ``key`` argument is not initialized
-status codes are not not defined by this specification.
+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
@@ -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
@@ -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
@@ -159,7 +234,7 @@ 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
@@ -167,7 +242,7 @@ Rejected Ideas
 
 * 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
@@ -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.
 
 
@@ -191,8 +266,10 @@ This document has been placed in the public domain.
 References and Footnotes
 ========================
 
-.. [1] https://bugs.python.org/msg116292
+.. [1] http://bugs.python.org/issue25658
-.. [2] http://bugs.python.org/file45548/configure-pthread_key_t.patch
+.. [2] https://bugs.python.org/msg116292
-.. [3] http://bugs.python.org/file44269/issue25658-1.patch
+.. [3] https://msdn.microsoft.com/en-us/library/windows/desktop/ms686801(v=vs.85).aspx
-.. [4] http://bugs.python.org/file44303/key-constant-time.diff
+.. [4] http://bugs.python.org/file45548/configure-pthread_key_t.patch
-.. [5] http://bugs.python.org/file45763/pythread-tss.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