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

[Masayuki YAMAMOTO]

2016-12-31 16:42 GMT+09:00 Nick Coghlan :

> On 31 December 2016 at 08:24, Masayuki YAMAMOTO  > wrote:
>
>> I have read the discussion and I'm sure that use structure as Py_tss_t
>> instead of platform-specific data type. Just as Steve said that Py_tss_t
>> should be genuinely treated as an opaque type, the key state checking
>> should provide macros or inline functions with name like
>> PyThread_tss_is_created. Well, I'd resolve the specification a bit more :)
>>
>> If PyThread_tss_create is called with the created key, it is no-op but
>> which the function should succeed or fail? In my opinion, It is better to
>> return a failure because it is a high possibility that the code is
>> incorrect for multiple callings of PyThread_tss_create for One key.
>>
>
> That's not what we currently do for the EnsureGIL autoTLS key and the
> tracemalloc key though - the reentrant key creation is part of
> "create-if-needed" flows where the key creation is silently skipped if the
> key already exists.
>
> Changing that would require some further research into how we ended up
> with the current approach, while carrying it over into the new API design
> would be the default option.
>

Yes, as you pointed out, my suggestion changes API semantics and not
inherit "create-if-needed". I confirmed again codes...current approach has
enough to work and I've not found strong benefit to change the semantics.
So I agree with you and withdraw my suggestion. Well, I'm going to update
patch based on the result.

Best regards,
Masayuki
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Masayuki YAMAMOTO]

I have read the discussion and I'm sure that use structure as Py_tss_t
instead of platform-specific data type. Just as Steve said that Py_tss_t
should be genuinely treated as an opaque type, the key state checking
should provide macros or inline functions with name like
PyThread_tss_is_created. Well, I'd resolve the specification a bit more :)

If PyThread_tss_create is called with the created key, it is no-op but
which the function should succeed or fail? In my opinion, It is better to
return a failure because it is a high possibility that the code is
incorrect for multiple callings of PyThread_tss_create for One key.

In this opinion PyThread_tss_is_created should return a value as follows:
(A) False while from after defining with Py_tss_NEED_INIT to before calling
PyThread_tss_create
(B) True after calling PyThread_tss_create succeeded
(C) Unchanging before and after calling PyThread_tss_create failed
(D) False after calling PyThread_tss_delete regardless of timing
(E) For other functions, the return value of PyThread_tss_is_created does
not change before and after calling

I think that it is better to write a test about the state of the Py_tss_t.

Kind regards,
Masayuki

2016-12-31 2:38 GMT+09:00 Erik Bray :

> On Fri, Dec 30, 2016 at 5:05 PM, Nick Coghlan  wrote:
> > On 29 December 2016 at 22:12, Erik Bray  wrote:
> >>
> >> 1) CPython's TLS: Defines -1 as an uninitialized key (by fact of the
> >> implementation--that the keys are integers starting from zero)
> >> 2) pthreads: Does not definite an uninitialized default value for
> >> keys, for reasons described at [1] under "Non-Idempotent Data Key
> >> Creation".  I understand their reasoning, though I can't claim to know
> >> specifically what they mean when they say that some implementations
> >> would require the mutual-exclusion to be performed on
> >> pthread_getspecific() as well.  I don't know that it applies here.
> >
> >
> > That section is a little weird, as they describe two requests (one for a
> > known-NULL default value, the other for implicit synchronisation of key
> > creation to prevent race conditions), and only provide the justification
> for
> > rejecting one of them (the second one).
>
> Right, that is confusing to me as well. I'm guessing the reason for
> rejecting the first is in part a way to force us to recognize the
> second issue.
>
> > If I've understood correctly, the situation they're worried about there
> is
> > that pthread_key_create() has to be called at least once-per-process, but
> > must be called before *any* call to pthread_getspecific or
> > pthread_setspecific for a given key. If you do "implicit init" rather
> than
> > requiring the use of an explicit mechanism like pthread_once (or our own
> > Py_Initialize and module import locks), then you may take a small
> > performance hit as either *every* thread then has to call
> > pthread_key_create() to ensure the key exists before using it, or else
> > pthread_getspecific() and pthread_setspecific() have to become
> potentially
> > blocking calls. Neither of those is desirable, so it makes sense to leave
> > that part of the problem to the API client.
> >
> > In our case, we don't want the implicit synchronisation, we just want the
> > known-NULL default value so the "Is it already set?" check can be moved
> > inside the library function.
>
> Okay, we're on the same page here then.  I just wanted to make sure
> there wasn't anything else I was missing in Python's case.
>
> >> 3) windows: The return value of TlsAlloc() is a DWORD (unsigned int)
> >> and [2] states that its value should be opaque.
> >>
> >> So in principle we can cover all cases with an opaque struct that
> >> contains, as its first member, an is_initialized flag.  The tricky
> >> part is how to initialize the rest of the struct (containing the
> >> underlying implementation-specific key).  For 1) and 3) it doesn't
> >> matter--it can just be zero.  For 2) it's trickier because there's no
> >> defined constant value to initialize a pthread_key_t to.
> >>
> >> Per Nick's suggestion this can be worked around by relying on C99's
> >> initialization semantics. Per [3] section 6.7.8, clause 21:
> >>
> >> """
> >> If there are fewer initializers in a brace-enclosed list than there
> >> are elements or members of an aggregate, or fewer characters in a
> >> string literal used to initialize an array of known size than there
> >> are elements in the array, the remainder of the aggregate shall be
> >> initialized implicitly the same as objects that have static storage
> >> duration.
> >> """
> >>
> >> How objects with static storage are initialized is described in the
> >> previous page under clause 10, but in practice it boils down to what
> >> you would expect: Everything is initialized to zero, including nested
> >> structs and arrays.
> >>
> >> So as long as we can use this feature of C99 then I think that's the
> >> best approach.
> >
> >

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

[Erik Bray]

On Fri, Dec 30, 2016 at 5:05 PM, Nick Coghlan  wrote:
> On 29 December 2016 at 22:12, Erik Bray  wrote:
>>
>> 1) CPython's TLS: Defines -1 as an uninitialized key (by fact of the
>> implementation--that the keys are integers starting from zero)
>> 2) pthreads: Does not definite an uninitialized default value for
>> keys, for reasons described at [1] under "Non-Idempotent Data Key
>> Creation".  I understand their reasoning, though I can't claim to know
>> specifically what they mean when they say that some implementations
>> would require the mutual-exclusion to be performed on
>> pthread_getspecific() as well.  I don't know that it applies here.
>
>
> That section is a little weird, as they describe two requests (one for a
> known-NULL default value, the other for implicit synchronisation of key
> creation to prevent race conditions), and only provide the justification for
> rejecting one of them (the second one).

Right, that is confusing to me as well. I'm guessing the reason for
rejecting the first is in part a way to force us to recognize the
second issue.

> If I've understood correctly, the situation they're worried about there is
> that pthread_key_create() has to be called at least once-per-process, but
> must be called before *any* call to pthread_getspecific or
> pthread_setspecific for a given key. If you do "implicit init" rather than
> requiring the use of an explicit mechanism like pthread_once (or our own
> Py_Initialize and module import locks), then you may take a small
> performance hit as either *every* thread then has to call
> pthread_key_create() to ensure the key exists before using it, or else
> pthread_getspecific() and pthread_setspecific() have to become potentially
> blocking calls. Neither of those is desirable, so it makes sense to leave
> that part of the problem to the API client.
>
> In our case, we don't want the implicit synchronisation, we just want the
> known-NULL default value so the "Is it already set?" check can be moved
> inside the library function.

Okay, we're on the same page here then.  I just wanted to make sure
there wasn't anything else I was missing in Python's case.

>> 3) windows: The return value of TlsAlloc() is a DWORD (unsigned int)
>> and [2] states that its value should be opaque.
>>
>> So in principle we can cover all cases with an opaque struct that
>> contains, as its first member, an is_initialized flag.  The tricky
>> part is how to initialize the rest of the struct (containing the
>> underlying implementation-specific key).  For 1) and 3) it doesn't
>> matter--it can just be zero.  For 2) it's trickier because there's no
>> defined constant value to initialize a pthread_key_t to.
>>
>> Per Nick's suggestion this can be worked around by relying on C99's
>> initialization semantics. Per [3] section 6.7.8, clause 21:
>>
>> """
>> If there are fewer initializers in a brace-enclosed list than there
>> are elements or members of an aggregate, or fewer characters in a
>> string literal used to initialize an array of known size than there
>> are elements in the array, the remainder of the aggregate shall be
>> initialized implicitly the same as objects that have static storage
>> duration.
>> """
>>
>> How objects with static storage are initialized is described in the
>> previous page under clause 10, but in practice it boils down to what
>> you would expect: Everything is initialized to zero, including nested
>> structs and arrays.
>>
>> So as long as we can use this feature of C99 then I think that's the
>> best approach.
>
>
>
> I checked PEP 7 to see exactly which features we've added to the approved C
> dialect, and designated initialisers are already on the list:
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
>
> So I believe that would allow the initializer to be declared as something
> like:
>
> #define Py_tss_NEEDS_INIT {.is_initialized = false}

Great!  One could argue about whether or not the designated
initializer syntax also incorporates omitted fields, but it would seem
strange to insist that it doesn't.

Have a happy new year,

Erik
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 29 December 2016 at 22:12, Erik Bray  wrote:

> 1) CPython's TLS: Defines -1 as an uninitialized key (by fact of the
> implementation--that the keys are integers starting from zero)
> 2) pthreads: Does not definite an uninitialized default value for
> keys, for reasons described at [1] under "Non-Idempotent Data Key
> Creation".  I understand their reasoning, though I can't claim to know
> specifically what they mean when they say that some implementations
> would require the mutual-exclusion to be performed on
> pthread_getspecific() as well.  I don't know that it applies here.
>

That section is a little weird, as they describe two requests (one for a
known-NULL default value, the other for implicit synchronisation of key
creation to prevent race conditions), and only provide the justification
for rejecting one of them (the second one).

If I've understood correctly, the situation they're worried about there is
that pthread_key_create() has to be called at least once-per-process, but
must be called before *any* call to pthread_getspecific or
pthread_setspecific for a given key. If you do "implicit init" rather than
requiring the use of an explicit mechanism like pthread_once (or our own
Py_Initialize and module import locks), then you may take a small
performance hit as either *every* thread then has to call
pthread_key_create() to ensure the key exists before using it, or else
pthread_getspecific() and pthread_setspecific() have to become potentially
blocking calls. Neither of those is desirable, so it makes sense to leave
that part of the problem to the API client.

In our case, we don't want the implicit synchronisation, we just want the
known-NULL default value so the "Is it already set?" check can be moved
inside the library function.


> 3) windows: The return value of TlsAlloc() is a DWORD (unsigned int)
> and [2] states that its value should be opaque.
>
> So in principle we can cover all cases with an opaque struct that
> contains, as its first member, an is_initialized flag.  The tricky
> part is how to initialize the rest of the struct (containing the
> underlying implementation-specific key).  For 1) and 3) it doesn't
> matter--it can just be zero.  For 2) it's trickier because there's no
> defined constant value to initialize a pthread_key_t to.
>
> Per Nick's suggestion this can be worked around by relying on C99's
> initialization semantics. Per [3] section 6.7.8, clause 21:
>
> """
> If there are fewer initializers in a brace-enclosed list than there
> are elements or members of an aggregate, or fewer characters in a
> string literal used to initialize an array of known size than there
> are elements in the array, the remainder of the aggregate shall be
> initialized implicitly the same as objects that have static storage
> duration.
> """
>
> How objects with static storage are initialized is described in the
> previous page under clause 10, but in practice it boils down to what
> you would expect: Everything is initialized to zero, including nested
> structs and arrays.
>
> So as long as we can use this feature of C99 then I think that's the
> best approach.
>


I checked PEP 7 to see exactly which features we've added to the approved C
dialect, and designated initialisers are already on the list:
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

So I believe that would allow the initializer to be declared as something
like:

#define Py_tss_NEEDS_INIT {.is_initialized = false}

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Dec 21, 2016 at 5:07 PM, Nick Coghlan  wrote:
> On 21 December 2016 at 20:01, Erik Bray  wrote:
>>
>> On Wed, Dec 21, 2016 at 2:10 AM, Nick Coghlan  wrote:
>> > Option 2: Similar to option 1, but using a custom type alias, rather
>> > than
>> > using a C99 bool directly
>> >
>> > The closest API we have to these semantics at the moment would be
>> > PyGILState_Ensure, so the following API naming might work for option 2:
>> >
>> > Py_ensure_t
>> > Py_ENSURE_NEEDS_INIT
>> > Py_ENSURE_INITIALIZED
>> >
>> > Respectively, these would just be aliases for bool, false, and true.
>> >
>> > And then modify the proposed PyThread_tss_create and PyThread_tss_delete
>> > APIs to accept a "Py_ensure_t *init_flag" in addition to their current
>> > arguments.
>>
>> That all sounds good--between the two option 2 looks a bit more explicit.
>>
>> Though what about this?  Rather than adding another type, the original
>> proposal could be changed slightly so that Py_tss_t *is* partially
>> defined as a struct consisting of a bool, with whatever the native TLS
>> key is.   E.g.
>>
>> typedef struct {
>> bool init_flag;
>> #if defined(_POSIX_THREADS)
>> pthreat_key_t key;
>> #elif defined (NT_THREADS)
>> DWORD key;
>> /* etc... */
>> } Py_tss_t;
>>
>> Then it's just taking Masayuki's original patch, with the global bool
>> variables, and formalizing that by combining the initialized flag with
>> the key, and requiring the semantics you described above for
>> PyThread_tss_create/delete.
>>
>> For Python's purposes it seems like this might be good enough, with
>> the more general purpose pthread_once-like functionality not required.
>
>
> Aye, I also thought of that approach, but talked myself out of it since
> there's no definable default value for pthread_key_t. However, C99 partial
> initialisation may deal with that for us (by zeroing the memory without
> actually assigning a typed value to it), and if it does, I agree it would be
> better to handle the initialisation flag automatically rather than requiring
> callers to do it.

I think I understand what you're saying here...  To be clear, let me
enumerate the three currently supported cases and how they're
affected:

1) CPython's TLS: Defines -1 as an uninitialized key (by fact of the
implementation--that the keys are integers starting from zero)
2) pthreads: Does not definite an uninitialized default value for
keys, for reasons described at [1] under "Non-Idempotent Data Key
Creation".  I understand their reasoning, though I can't claim to know
specifically what they mean when they say that some implementations
would require the mutual-exclusion to be performed on
pthread_getspecific() as well.  I don't know that it applies here.
3) windows: The return value of TlsAlloc() is a DWORD (unsigned int)
and [2] states that its value should be opaque.

So in principle we can cover all cases with an opaque struct that
contains, as its first member, an is_initialized flag.  The tricky
part is how to initialize the rest of the struct (containing the
underlying implementation-specific key).  For 1) and 3) it doesn't
matter--it can just be zero.  For 2) it's trickier because there's no
defined constant value to initialize a pthread_key_t to.

Per Nick's suggestion this can be worked around by relying on C99's
initialization semantics. Per [3] section 6.7.8, clause 21:

"""
If there are fewer initializers in a brace-enclosed list than there
are elements or members of an aggregate, or fewer characters in a
string literal used to initialize an array of known size than there
are elements in the array, the remainder of the aggregate shall be
initialized implicitly the same as objects that have static storage
duration.
"""

How objects with static storage are initialized is described in the
previous page under clause 10, but in practice it boils down to what
you would expect: Everything is initialized to zero, including nested
structs and arrays.

So as long as we can use this feature of C99 then I think that's the
best approach.



[1] 
http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html
[2] 
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686801(v=vs.85).aspx
[3] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Steve Dower]

Right. Platforms that have a defined invalid value don't need the struct, and 
so they can define the type differently. It just means we also need to provide 
a macro for testing whether it's been created or not, and users should 
genuinely treat the value as opaque.

Cheers,
Steve

Top-posted from my Windows Phone

-Original Message-
From: "Masayuki YAMAMOTO" <ma3yuki.8mam...@gmail.com>
Sent: ‎12/‎23/‎2016 16:34
To: "Erik Bray" <erik.m.b...@gmail.com>
Cc: "python-ideas@python.org" <python-ideas@python.org>
Subject: Re: [Python-ideas] New PyThread_tss_ C-API for CPython

2016-12-21 19:01 GMT+09:00 Erik Bray <erik.m.b...@gmail.com>:

On Wed, Dec 21, 2016 at 2:10 AM, Nick Coghlan <ncogh...@gmail.com> wrote:
> Ouch, I'd missed that, and I agree it's not a negligible implementation
> detail - there are definitely applications embedding CPython out there that
> rely on being able to run multiple Initialize/Finalize cycles in the same
> process and have everything "just work". It also means using the
> "PyThread_*" prefix for the initialisation tracking aspect would be
> misleading, since the life cycle details are:
>
> 1. Create the key for the first time if it has never been previously set in
> the process
> 2. Destroy and reinit if Py_Finalize gets called
> 3. Destroy and reinit if a new subprocess is forked
>
> It also means we can't use pthread_once even in the pthread TLS
> implementation, since it doesn't provide those semantics.
>
> So I see two main alternatives here.
>
> Option 1: Modify the proposed PyThread_tss_create and PyThread_tss_delete
> APIs to accept a "bool *init_flag" pointer in addition to their current
> arguments.
>
> If *init_flag is true, then PyThread_tss_create is a no-op, otherwise it
> sets the flag to true after creating the key.
> If *init_flag is false, then PyThread_tss_delete is a no-op, otherwise it
> sets the flag to false after deleting the key.
>
> Option 2: Similar to option 1, but using a custom type alias, rather than
> using a C99 bool directly
>
> The closest API we have to these semantics at the moment would be
> PyGILState_Ensure, so the following API naming might work for option 2:
>
> Py_ensure_t
> Py_ENSURE_NEEDS_INIT
> Py_ENSURE_INITIALIZED
>
> Respectively, these would just be aliases for bool, false, and true.
>
> And then modify the proposed PyThread_tss_create and PyThread_tss_delete
> APIs to accept a "Py_ensure_t *init_flag" in addition to their current
> arguments.


That all sounds good--between the two option 2 looks a bit more explicit.

Though what about this?  Rather than adding another type, the original
proposal could be changed slightly so that Py_tss_t *is* partially
defined as a struct consisting of a bool, with whatever the native TLS
key is.   E.g.

typedef struct {
bool init_flag;
#if defined(_POSIX_THREADS)
pthreat_key_t key;
#elif defined (NT_THREADS)
DWORD key;
/* etc... */
} Py_tss_t;

Then it's just taking Masayuki's original patch, with the global bool
variables, and formalizing that by combining the initialized flag with
the key, and requiring the semantics you described above for
PyThread_tss_create/delete.

For Python's purposes it seems like this might be good enough, with
the more general purpose pthread_once-like functionality not required.

Best,
Erik

Above mentioned, In currently TLS API, the thread key uses -1 as defined 
invalid value. If new TLS API inherits the specifications that the key requires 
defined invalid value, putting key and flag into one structure seems correct as 
semantics. In this case, I think TLS API should supply the defined invalid 
value (like PTHREAD_ONCE_INIT) to API users.

Moreover, the structure has an opportunity to assert that the thread key type 
is the opaque using field name. I think to the suggestion that has effect to 
improve the understandability of the API because good field name can give that 
reading and writing to the key seems to be incorrect (even if API users don't 
read the precautionary statement).


Have a nice holiday!

Masayuki___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Masayuki YAMAMOTO]

2016-12-21 19:01 GMT+09:00 Erik Bray :

> On Wed, Dec 21, 2016 at 2:10 AM, Nick Coghlan  wrote:
> > Ouch, I'd missed that, and I agree it's not a negligible implementation
> > detail - there are definitely applications embedding CPython out there
> that
> > rely on being able to run multiple Initialize/Finalize cycles in the same
> > process and have everything "just work". It also means using the
> > "PyThread_*" prefix for the initialisation tracking aspect would be
> > misleading, since the life cycle details are:
> >
> > 1. Create the key for the first time if it has never been previously set
> in
> > the process
> > 2. Destroy and reinit if Py_Finalize gets called
> > 3. Destroy and reinit if a new subprocess is forked
> >
> > It also means we can't use pthread_once even in the pthread TLS
> > implementation, since it doesn't provide those semantics.
> >
> > So I see two main alternatives here.
> >
> > Option 1: Modify the proposed PyThread_tss_create and PyThread_tss_delete
> > APIs to accept a "bool *init_flag" pointer in addition to their current
> > arguments.
> >
> > If *init_flag is true, then PyThread_tss_create is a no-op, otherwise it
> > sets the flag to true after creating the key.
> > If *init_flag is false, then PyThread_tss_delete is a no-op, otherwise it
> > sets the flag to false after deleting the key.
> >
> > Option 2: Similar to option 1, but using a custom type alias, rather than
> > using a C99 bool directly
> >
> > The closest API we have to these semantics at the moment would be
> > PyGILState_Ensure, so the following API naming might work for option 2:
> >
> > Py_ensure_t
> > Py_ENSURE_NEEDS_INIT
> > Py_ENSURE_INITIALIZED
> >
> > Respectively, these would just be aliases for bool, false, and true.
> >
> > And then modify the proposed PyThread_tss_create and PyThread_tss_delete
> > APIs to accept a "Py_ensure_t *init_flag" in addition to their current
> > arguments.
>
> That all sounds good--between the two option 2 looks a bit more explicit.
>
> Though what about this?  Rather than adding another type, the original
> proposal could be changed slightly so that Py_tss_t *is* partially
> defined as a struct consisting of a bool, with whatever the native TLS
> key is.   E.g.
>
> typedef struct {
> bool init_flag;
> #if defined(_POSIX_THREADS)
> pthreat_key_t key;
> #elif defined (NT_THREADS)
> DWORD key;
> /* etc... */
> } Py_tss_t;
>
> Then it's just taking Masayuki's original patch, with the global bool
> variables, and formalizing that by combining the initialized flag with
> the key, and requiring the semantics you described above for
> PyThread_tss_create/delete.
>
> For Python's purposes it seems like this might be good enough, with
> the more general purpose pthread_once-like functionality not required.
>
> Best,
> Erik


Above mentioned, In currently TLS API, the thread key uses -1 as defined
invalid value. If new TLS API inherits the specifications that the key
requires defined invalid value, putting key and flag into one structure
seems correct as semantics. In this case, I think TLS API should supply the
defined invalid value (like PTHREAD_ONCE_INIT) to API users.
Moreover, the structure has an opportunity to assert that the thread key
type is the opaque using field name. I think to the suggestion that has
effect to improve the understandability of the API because good field name
can give that reading and writing to the key seems to be incorrect (even if
API users don't read the precautionary statement).

Have a nice holiday!
Masayuki
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 21 December 2016 at 20:01, Erik Bray  wrote:

> On Wed, Dec 21, 2016 at 2:10 AM, Nick Coghlan  wrote:
> > Option 2: Similar to option 1, but using a custom type alias, rather than
> > using a C99 bool directly
> >
> > The closest API we have to these semantics at the moment would be
> > PyGILState_Ensure, so the following API naming might work for option 2:
> >
> > Py_ensure_t
> > Py_ENSURE_NEEDS_INIT
> > Py_ENSURE_INITIALIZED
> >
> > Respectively, these would just be aliases for bool, false, and true.
> >
> > And then modify the proposed PyThread_tss_create and PyThread_tss_delete
> > APIs to accept a "Py_ensure_t *init_flag" in addition to their current
> > arguments.
>
> That all sounds good--between the two option 2 looks a bit more explicit.
>
> Though what about this?  Rather than adding another type, the original
> proposal could be changed slightly so that Py_tss_t *is* partially
> defined as a struct consisting of a bool, with whatever the native TLS
> key is.   E.g.
>
> typedef struct {
> bool init_flag;
> #if defined(_POSIX_THREADS)
> pthreat_key_t key;
> #elif defined (NT_THREADS)
> DWORD key;
> /* etc... */
> } Py_tss_t;
>
> Then it's just taking Masayuki's original patch, with the global bool
> variables, and formalizing that by combining the initialized flag with
> the key, and requiring the semantics you described above for
> PyThread_tss_create/delete.
>
> For Python's purposes it seems like this might be good enough, with
> the more general purpose pthread_once-like functionality not required.
>

Aye, I also thought of that approach, but talked myself out of it since
there's no definable default value for pthread_key_t. However, C99 partial
initialisation may deal with that for us (by zeroing the memory without
actually assigning a typed value to it), and if it does, I agree it would
be better to handle the initialisation flag automatically rather than
requiring callers to do it.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Dec 21, 2016 at 11:01 AM, Erik Bray  wrote:
> That all sounds good--between the two option 2 looks a bit more explicit.
>
> Though what about this?  Rather than adding another type, the original
> proposal could be changed slightly so that Py_tss_t *is* partially
> defined as a struct consisting of a bool, with whatever the native TLS
> key is.   E.g.
>
> typedef struct {
> bool init_flag;
> #if defined(_POSIX_THREADS)
> pthreat_key_t key;

*pthread_key_t* of course, though I wonder if that was a Freudian slip :)

> #elif defined (NT_THREADS)
> DWORD key;
> /* etc... */
> } Py_tss_t;
>
> Then it's just taking Masayuki's original patch, with the global bool
> variables, and formalizing that by combining the initialized flag with
> the key, and requiring the semantics you described above for
> PyThread_tss_create/delete.
>
> For Python's purposes it seems like this might be good enough, with
> the more general purpose pthread_once-like functionality not required.

Of course, that's not to say it might not be useful for some other
purpose, but then it's outside the scope of this discussion as long as
it isn't needed for TLS key initialization.
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Wed, Dec 21, 2016 at 2:10 AM, Nick Coghlan  wrote:
> On 21 December 2016 at 01:35, Masayuki YAMAMOTO 
> wrote:
>>
>> 2016-12-20 22:30 GMT+09:00 Erik Bray :
>>>
>>> This is probably an implementation detail, but ISTM that even with
>>> PyThread_call_once, it will be necessary to reset any used once_flags
>>> manually in PyOS_AfterFork, essentially for the same reason the
>>> autoTLSkey is reset there currently...
>>
>>
>> Deleting threads key is executed on *_Fini functions, but Py_FinalizeEx
>> function that calls *_Fini functions doesn't terminate CPython interpreter.
>> Furthermore, source comment and document have said description about
>> reinitialization after calling Py_FinalizeEx. [1] [2] That is to say there
>> is an implicit possible that is reinitialization contrary to name
>> "call_once" on a process level. Therefore, if CPython interpreter continues
>> to allow reinitialization, I'd suggest to rename the call_once API to avoid
>> misreading semantics. (for example, safe_init, check_init)
>
>
> Ouch, I'd missed that, and I agree it's not a negligible implementation
> detail - there are definitely applications embedding CPython out there that
> rely on being able to run multiple Initialize/Finalize cycles in the same
> process and have everything "just work". It also means using the
> "PyThread_*" prefix for the initialisation tracking aspect would be
> misleading, since the life cycle details are:
>
> 1. Create the key for the first time if it has never been previously set in
> the process
> 2. Destroy and reinit if Py_Finalize gets called
> 3. Destroy and reinit if a new subprocess is forked
>
> It also means we can't use pthread_once even in the pthread TLS
> implementation, since it doesn't provide those semantics.
>
> So I see two main alternatives here.
>
> Option 1: Modify the proposed PyThread_tss_create and PyThread_tss_delete
> APIs to accept a "bool *init_flag" pointer in addition to their current
> arguments.
>
> If *init_flag is true, then PyThread_tss_create is a no-op, otherwise it
> sets the flag to true after creating the key.
> If *init_flag is false, then PyThread_tss_delete is a no-op, otherwise it
> sets the flag to false after deleting the key.
>
> Option 2: Similar to option 1, but using a custom type alias, rather than
> using a C99 bool directly
>
> The closest API we have to these semantics at the moment would be
> PyGILState_Ensure, so the following API naming might work for option 2:
>
> Py_ensure_t
> Py_ENSURE_NEEDS_INIT
> Py_ENSURE_INITIALIZED
>
> Respectively, these would just be aliases for bool, false, and true.
>
> And then modify the proposed PyThread_tss_create and PyThread_tss_delete
> APIs to accept a "Py_ensure_t *init_flag" in addition to their current
> arguments.

That all sounds good--between the two option 2 looks a bit more explicit.

Though what about this?  Rather than adding another type, the original
proposal could be changed slightly so that Py_tss_t *is* partially
defined as a struct consisting of a bool, with whatever the native TLS
key is.   E.g.

typedef struct {
bool init_flag;
#if defined(_POSIX_THREADS)
pthreat_key_t key;
#elif defined (NT_THREADS)
DWORD key;
/* etc... */
} Py_tss_t;

Then it's just taking Masayuki's original patch, with the global bool
variables, and formalizing that by combining the initialized flag with
the key, and requiring the semantics you described above for
PyThread_tss_create/delete.

For Python's purposes it seems like this might be good enough, with
the more general purpose pthread_once-like functionality not required.

Best,
Erik
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 21 December 2016 at 01:35, Masayuki YAMAMOTO 
wrote:

> 2016-12-20 22:30 GMT+09:00 Erik Bray :
>
>> This is probably an implementation detail, but ISTM that even with
>> PyThread_call_once, it will be necessary to reset any used once_flags
>> manually in PyOS_AfterFork, essentially for the same reason the
>> autoTLSkey is reset there currently...
>>
>
> Deleting threads key is executed on *_Fini functions, but Py_FinalizeEx
> function that calls *_Fini functions doesn't terminate CPython interpreter.
> Furthermore, source comment and document have said description about
> reinitialization after calling Py_FinalizeEx. [1] [2] That is to say there
> is an implicit possible that is reinitialization contrary to name
> "call_once" on a process level. Therefore, if CPython interpreter continues
> to allow reinitialization, I'd suggest to rename the call_once API to avoid
> misreading semantics. (for example, safe_init, check_init)
>

Ouch, I'd missed that, and I agree it's not a negligible implementation
detail - there are definitely applications embedding CPython out there that
rely on being able to run multiple Initialize/Finalize cycles in the same
process and have everything "just work". It also means using the
"PyThread_*" prefix for the initialisation tracking aspect would be
misleading, since the life cycle details are:

1. Create the key for the first time if it has never been previously set in
the process
2. Destroy and reinit if Py_Finalize gets called
3. Destroy and reinit if a new subprocess is forked

It also means we can't use pthread_once even in the pthread TLS
implementation, since it doesn't provide those semantics.

So I see two main alternatives here.

Option 1: Modify the proposed PyThread_tss_create and PyThread_tss_delete
APIs to accept a "bool *init_flag" pointer in addition to their current
arguments.

If *init_flag is true, then PyThread_tss_create is a no-op, otherwise it
sets the flag to true after creating the key.
If *init_flag is false, then PyThread_tss_delete is a no-op, otherwise it
sets the flag to false after deleting the key.

Option 2: Similar to option 1, but using a custom type alias, rather than
using a C99 bool directly

The closest API we have to these semantics at the moment would be
PyGILState_Ensure, so the following API naming might work for option 2:

Py_ensure_t
Py_ENSURE_NEEDS_INIT
Py_ENSURE_INITIALIZED

Respectively, these would just be aliases for bool, false, and true.

And then modify the proposed PyThread_tss_create and PyThread_tss_delete
APIs to accept a "Py_ensure_t *init_flag" in addition to their current
arguments.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Masayuki YAMAMOTO]

2016-12-20 22:30 GMT+09:00 Erik Bray :

> This is probably an implementation detail, but ISTM that even with
> PyThread_call_once, it will be necessary to reset any used once_flags
> manually in PyOS_AfterFork, essentially for the same reason the
> autoTLSkey is reset there currently...
>

Deleting threads key is executed on *_Fini functions, but Py_FinalizeEx
function that calls *_Fini functions doesn't terminate CPython interpreter.
Furthermore, source comment and document have said description about
reinitialization after calling Py_FinalizeEx. [1] [2] That is to say there
is an implicit possible that is reinitialization contrary to name
"call_once" on a process level. Therefore, if CPython interpreter continues
to allow reinitialization, I'd suggest to rename the call_once API to avoid
misreading semantics. (for example, safe_init, check_init)

Best regards,
Masayuki

[1] https://hg.python.org/cpython/file/default/Python/pylifecycle.c#l170
[2] https://docs.python.org/dev/c-api/init.html#c.Py_FinalizeEx
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 20 December 2016 at 00:53, Erik Bray  wrote:

> On Mon, Dec 19, 2016 at 3:45 PM, Erik Bray  wrote:
> >> Likewise - we know the status quo isn't right, and the proposed change
> >> addresses that. In reviewing the patch on the tracker, the one downside
> I've
> >> found is that due to "pthread_key_t" being an opaque type with no
> defined
> >> sentinel, the consuming code in _tracemalloc.c and pystate.c needed to
> add
> >> separate boolean flag variables to track whether or not the key had been
> >> created. (The pthread examples at
> >> http://pubs.opengroup.org/onlinepubs/009695399/
> functions/pthread_key_create.html
> >> use pthread_once for a similar effect)
> >>
> >> I don't see any obvious way around that either, as even using a small
> struct
> >> for native pthread TLS keys would still face the problem of how to
> >> initialise the pthread_key_t field.
> >
> > Hmm...fair point that it's not pretty.  One way around it, albeit
> > requiring more work/complexity, would be to extend this proposal to
> > add a new function analogous to pthread_once--say--PyThread_call_once,
> > and an associated Py_once_flag_t
>
> Oops--fat-fingered a 'send' command before I finished.
>
> So  workaround would be to add a PyThread_call_once function,
> analogous to pthread_once.  Yet another interface one needs to
> implement for a native thread implementation, but not too hard either.
> For pthreads there's already an obvious analogue that can be wrapped
> directly.  For other platforms that don't have a direct analogue a
> (naive) implementation is still fairly simple: All you need in
> Py_once_flag_t is a boolean flag with an associated mutex, and a
> sentinel value analogous to PTHREAD_ONCE_INIT.
>

Yeah, I think I'd prefer that - it aligns nicely with the way pthreads are
defined, and means we can be more prescriptive about how to use the new API
correctly for key declarations (we're currently a bit vague about exactly
how to handle that in the current TLS API).

With that addition, I think it will be worth turning your initial post here
into a PR to the peps repo, though - not to resolve any particular
controversy, but rather as an easier to find reference for the design
rationale than a mailing list thread or a tracker issue.

(I'd also be happy to volunteer as BDFL-Delegate, since I'm already
reviewing the patch on the tracker)

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Mon, Dec 19, 2016 at 3:45 PM, Erik Bray  wrote:
> On Mon, Dec 19, 2016 at 1:11 PM, Nick Coghlan  wrote:
>> On 17 December 2016 at 03:51, Antoine Pitrou  wrote:
>>>
>>> On Fri, 16 Dec 2016 13:07:46 +0100
>>> Erik Bray  wrote:
>>> > Greetings all,
>>> >
>>> > I wanted to bring attention to an issue that's been languishing on the
>>> > bug tracker since last year, which I think would best be addressed by
>>> > changes to CPython's C-API.  The original issue is at
>>> > http://bugs.python.org/issue25658, but I have made an effort below in
>>> > a sort of proto-PEP to summarize the problem and the proposed
>>> > solution.
>>> >
>>> > I haven't written this up in the proper PEP format because I want to
>>> > see if the idea has some broader support first, and it's also not
>>> > clear to me whether C-API changes (especially to undocumented APIs)
>>> > even require their own PEP.
>>>
>>> This is a nice detailed write-up and I'm in favour of the proposal.
>>
>>
>> Likewise - we know the status quo isn't right, and the proposed change
>> addresses that. In reviewing the patch on the tracker, the one downside I've
>> found is that due to "pthread_key_t" being an opaque type with no defined
>> sentinel, the consuming code in _tracemalloc.c and pystate.c needed to add
>> separate boolean flag variables to track whether or not the key had been
>> created. (The pthread examples at
>> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html
>> use pthread_once for a similar effect)
>>
>> I don't see any obvious way around that either, as even using a small struct
>> for native pthread TLS keys would still face the problem of how to
>> initialise the pthread_key_t field.
>
> Hmm...fair point that it's not pretty.  One way around it, albeit
> requiring more work/complexity, would be to extend this proposal to
> add a new function analogous to pthread_once--say--PyThread_call_once,
> and an associated Py_once_flag_t

Oops--fat-fingered a 'send' command before I finished.

So  workaround would be to add a PyThread_call_once function,
analogous to pthread_once.  Yet another interface one needs to
implement for a native thread implementation, but not too hard either.
For pthreads there's already an obvious analogue that can be wrapped
directly.  For other platforms that don't have a direct analogue a
(naive) implementation is still fairly simple: All you need in
Py_once_flag_t is a boolean flag with an associated mutex, and a
sentinel value analogous to PTHREAD_ONCE_INIT.

Best,
Erik
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Mon, Dec 19, 2016 at 1:11 PM, Nick Coghlan  wrote:
> On 17 December 2016 at 03:51, Antoine Pitrou  wrote:
>>
>> On Fri, 16 Dec 2016 13:07:46 +0100
>> Erik Bray  wrote:
>> > Greetings all,
>> >
>> > I wanted to bring attention to an issue that's been languishing on the
>> > bug tracker since last year, which I think would best be addressed by
>> > changes to CPython's C-API.  The original issue is at
>> > http://bugs.python.org/issue25658, but I have made an effort below in
>> > a sort of proto-PEP to summarize the problem and the proposed
>> > solution.
>> >
>> > I haven't written this up in the proper PEP format because I want to
>> > see if the idea has some broader support first, and it's also not
>> > clear to me whether C-API changes (especially to undocumented APIs)
>> > even require their own PEP.
>>
>> This is a nice detailed write-up and I'm in favour of the proposal.
>
>
> Likewise - we know the status quo isn't right, and the proposed change
> addresses that. In reviewing the patch on the tracker, the one downside I've
> found is that due to "pthread_key_t" being an opaque type with no defined
> sentinel, the consuming code in _tracemalloc.c and pystate.c needed to add
> separate boolean flag variables to track whether or not the key had been
> created. (The pthread examples at
> http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html
> use pthread_once for a similar effect)
>
> I don't see any obvious way around that either, as even using a small struct
> for native pthread TLS keys would still face the problem of how to
> initialise the pthread_key_t field.

Hmm...fair point that it's not pretty.  One way around it, albeit
requiring more work/complexity, would be to extend this proposal to
add a new function analogous to pthread_once--say--PyThread_call_once,
and an associated Py_once_flag_t
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Nick Coghlan]

On 17 December 2016 at 03:51, Antoine Pitrou  wrote:

> On Fri, 16 Dec 2016 13:07:46 +0100
> Erik Bray  wrote:
> > Greetings all,
> >
> > I wanted to bring attention to an issue that's been languishing on the
> > bug tracker since last year, which I think would best be addressed by
> > changes to CPython's C-API.  The original issue is at
> > http://bugs.python.org/issue25658, but I have made an effort below in
> > a sort of proto-PEP to summarize the problem and the proposed
> > solution.
> >
> > I haven't written this up in the proper PEP format because I want to
> > see if the idea has some broader support first, and it's also not
> > clear to me whether C-API changes (especially to undocumented APIs)
> > even require their own PEP.
>
> This is a nice detailed write-up and I'm in favour of the proposal.
>

Likewise - we know the status quo isn't right, and the proposed change
addresses that. In reviewing the patch on the tracker, the one downside
I've found is that due to "pthread_key_t" being an opaque type with no
defined sentinel, the consuming code in _tracemalloc.c and pystate.c needed
to add separate boolean flag variables to track whether or not the key had
been created. (The pthread examples at
http://pubs.opengroup.org/onlinepubs/009695399/functions/pthread_key_create.html
use pthread_once for a similar effect)

I don't see any obvious way around that either, as even using a small
struct for native pthread TLS keys would still face the problem of how to
initialise the pthread_key_t field.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Sat, Dec 17, 2016 at 8:21 AM, Stephen J. Turnbull
 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
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Erik Bray]

On Sun, Dec 18, 2016 at 12:10 AM, Masayuki YAMAMOTO
 wrote:
> 2016-12-17 18:35 GMT+09:00 Stephen J. Turnbull
> :
>>
>> 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").
>
>
> PyThread_create_key has required user to check the return value since when
> key creation fails, returns -1 instead of valid key value.  Therefore, my
> patch changes PyThread_create_key that always return -1 on platforms that
> cannot cast key to int safely and current API never return valid key value
> to these platforms.  Its advantage to not change function specifications and
> no effect on supported platforms. Hence, this is reason that doesn't raise
> any exception on the API.
>
> (2) of ideas can enable current API on specific-platforms. If it's simple,
> I'd have liked to select it.  However, work that brings current API using
> native TLS to specific-platforms brings duplication implementation that
> manages keys, and it's ugly (same reason for Erik's draft, the last item of
> Rejected Ideas).  Thus, I gave up to keep feature and decided to implement
> "no-op", delegate error handling to API users.

Yep--I think it speaks to the sensibleness of that decision that I
pretty much read your mind :)
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Masayuki YAMAMOTO]

2016-12-17 18:35 GMT+09:00 Stephen J. Turnbull :

> 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").


PyThread_create_key has required user to check the return value since when
key creation fails, returns -1 instead of valid key value.  Therefore, my
patch changes PyThread_create_key that always return -1 on platforms that
cannot cast key to int safely and current API never return valid key value
to these platforms.  Its advantage to not change function specifications
and no effect on supported platforms. Hence, this is reason that doesn't
raise any exception on the API.

(2) of ideas can enable current API on specific-platforms. If it's simple,
I'd have liked to select it.  However, work that brings current API using
native TLS to specific-platforms brings duplication implementation that
manages keys, and it's ugly (same reason for Erik's draft, the last item of
Rejected Ideas).  Thus, I gave up to keep feature and decided to implement
"no-op", delegate error handling to API users.

Kind regards,
Masayuki
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Masayuki YAMAMOTO]

Hi, I'm patch author,

I don't need to say anything for Erik's draft. I feel awesome that it has
been clearly to explain, especially for history of API and against PEP.
Thanks for great job, Erik!

Cheers,
Masayuki
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Chris Angelico]

On Fri, Dec 16, 2016 at 11:07 PM, Erik Bray  wrote:
> I haven't written this up in the proper PEP format because I want to
> see if the idea has some broader support first, and it's also not
> clear to me whether C-API changes (especially to undocumented APIs)
> even require their own PEP.
>

You're pretty close to proper PEP format. Like others, I don't have
enough knowledge of threading internals to speak to the technical side
of it, but this is a well-written proposal and I agree in principle
with tightening this up. The need for a PEP basically comes down to
whether or not it's going to be controversial; a PEP allows you to
hash out the details and then present a coherent proposal to Guido (or
his delegate) for final approval.

ChrisA
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Antoine Pitrou]

On Fri, 16 Dec 2016 13:07:46 +0100
Erik Bray  wrote:
> Greetings all,
> 
> I wanted to bring attention to an issue that's been languishing on the
> bug tracker since last year, which I think would best be addressed by
> changes to CPython's C-API.  The original issue is at
> http://bugs.python.org/issue25658, but I have made an effort below in
> a sort of proto-PEP to summarize the problem and the proposed
> solution.
> 
> I haven't written this up in the proper PEP format because I want to
> see if the idea has some broader support first, and it's also not
> clear to me whether C-API changes (especially to undocumented APIs)
> even require their own PEP.

This is a nice detailed write-up and I'm in favour of the proposal.

Regards

Antoine.


___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/

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

[Zachary Ware]

On Fri, Dec 16, 2016 at 6:07 AM, Erik Bray  wrote:
> Greetings all,
>
> I wanted to bring attention to an issue that's been languishing on the
> bug tracker since last year, which I think would best be addressed by
> changes to CPython's C-API.  The original issue is at
> http://bugs.python.org/issue25658, but I have made an effort below in
> a sort of proto-PEP to summarize the problem and the proposed
> solution.

I am not familiar enough with the threading implementation to be
anything more than moral support, but I am in favor of making some
change here.  This is a significant blocker to Cygwin support, which
is actually fairly close to being supportable.

-- 
Zach
___
Python-ideas mailing list
Python-ideas@python.org
https://mail.python.org/mailman/listinfo/python-ideas
Code of Conduct: http://python.org/psf/codeofconduct/