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

Stefan Behnel added the comment: Seems like this isn't trivial, so I created a new ticket for this. See issue 31828. -- ___ Python tracker

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

Stefan Behnel added the comment: It seems that there's a simpler way that uses a cast on the literal. I added a pull request. Looks simple enough to not merit its own ticket, I think. -- ___ Python tracker

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

Change by Stefan Behnel : -- pull_requests: +4029 ___ Python tracker ___ ___

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

Nick Coghlan added the comment: Stefan: I'd expect so, but that's best covered by a new RFE and an associated PR (I'm not sure what you mean from the text description, but I assume it will be obvious as C code) -- ___ Python

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

Stefan Behnel added the comment: Would it be possible to define Py_tss_NEEDS_INIT as a constant variable instead of a static initialiser? That would enable its use also for non-static initialisations. -- nosy: +scoder ___

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

Nick Coghlan added the comment: This has been merged now: https://github.com/python/cpython/commit/731e18901484c75b60167a06a0ba0719a6d4827d Thank you for the PEP & implementation! :) -- resolution: -> fixed stage: patch review -> resolved status: open -> closed

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

Masayuki Yamamoto added the comment: FYI, PEP 539 was accepted (see python-dev threads [1] [2]). The only thing missing is a reference implementation, next I complete it. [1] https://mail.python.org/pipermail/python-dev/2017-August/149091.html [2]

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

Masayuki Yamamoto added the comment: Since previous comment, I've studied the switch for show/hide implementation detail. As the result, I have understood the Py_BUILD_CORE macro hasn't been generally used for hiding implementation detail (and Py_LIMITED_API does the part). Therefore, I

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

Masayuki Yamamoto added the comment: oh, I found a mistake. - replace Py_LIMITED_API with Py_BUILD_CORE on Include/pythread.h Py_tss_t definition -- ___ Python tracker

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

Masayuki Yamamoto added the comment: Nick and Erik, thank you for the comments! I merged proposal into the PR. Well, I'm interested in the hide implementation detail for TSS API (lately, I've read the python-ideas thread "PEP: Hide implementation details in the C API" which Victor opened

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

Nick Coghlan added the comment: Nice :) With the legacy code cleanups merged, I'd say the next step would be to update the PEP with the simplified API and the explanation for why the removed functions are no longer needed (i.e. we're making native TSS support a hard dependency for 3.7+).

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

Masayuki Yamamoto added the comment: Hi, I attempted Nick's proposal and removed unused codes from TLS implementation (bpo-30279, bpo-30832). This change looks good to me (PR 1362). As the result, I think ready to be more slim API because the own implementation for TLS was removed. Therefore,

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

Nick Coghlan added the comment: Noting a design consideration that I only picked up in the latest PR review: when exposed as part of a non-opaque struct, the type used for TSS keys becomes part of Python's ABI, which means the API design in the PEP is going to have to handle making the struct

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

Masayuki Yamamoto added the comment: Victor, The tracemalloc module is not available on platforms that provides own TLS implementation that uses PyMem_* functions. In the own implementation, it occurs deadlock by recursion call in thread key search. PyOnceVar API handles extendable array by

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

Changes by Masayuki Yamamoto : -- pull_requests: +1472 ___ Python tracker ___ ___

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

STINNER Victor added the comment: Hi people working on the new TLS API: I would like your opinion on a related API, issue #29881: Add a new private API for "static C variables" (_PyStaticVar) to clear them at exit ! -- ___ Python tracker

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

Masayuki Yamamoto added the comment: Above said, I updated minor changes to the version 2 patch. Several codes have kept the words "thread local" and "TLS" because they have pointed programming method or other meanings, not CPython TLS API itself. (e.g. _decimal module) -- Added

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

Masayuki Yamamoto added the comment: I commented at the Rietveld, since I think that patch needs a bit more modification. * rename PyThread_ReInitTLS * update comments and messages that have explained CPython TLS API -- ___ Python tracker

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

Erik Bray added the comment: Thanks Masayuki for the updated patch, and especially for the additional test cases. Looking at the patch, it occurs to me that this solution seems to be the minimal needed to fix the issue that we were originally trying to fix, without adding too much additional

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

Masayuki Yamamoto added the comment: After Erik posted PEP 539 draft, we've discussed features of new API on the Python-ideas [*]. As the result of discussion, features of new API have been changed below points. Thus, I updated patch based on the result. 1. API uses opaque type because to

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

Changes by Nick Coghlan : -- nosy: +ncoghlan ___ Python tracker ___ ___ Python-bugs-list

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

Erik Bray added the comment: I'm still pretty happy with the previous patch, personally, since I don't need the tracemalloc module. But it seems like that should be fixed (or if nothing else that code in _tracemalloc.c should check Py_HAVE_NATIVE_TLS too). I like the idea of the new

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

Masayuki Yamamoto added the comment: I wrote a patch based on msg281227's idea. I confirmed to pass related tests (test_capi, test_threading and test_tracemalloc) on Cygwin x86 and Ubuntu 16.04 x86. This patch adds to change CPython about: 1. Avoid compile error on Currently TLS API.

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

Masayuki Yamamoto added the comment: Elik, Ed, I have overlooked tracemalloc module raises deadlock if apply the patch. I found out a source comment on Modules/_tracemalloc.c:161 /* If your OS does not provide native thread local storage, you can implement it manually using a lock.

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

Ed Schouten added the comment: Looks good to me! -- ___ Python tracker ___ ___ Python-bugs-list mailing list

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

Erik Bray added the comment: To me, Masayuki's patch is an acceptable work-around for the time being. I don't *like* it because the fact remains that native TLS can work on these platforms and falling back on Python's slower implementation isn't necessary. That said, the previous patch

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

Masayuki Yamamoto added the comment: I wrote a patch to avoid compile error for platforms that pthread_key_t is not integer. This patch changes to turn off Py_HAVE_NATIVE_TLS if pthread_key_t is not integer. Hence the platforms use TLS functions implemented by CPython self. And, I would

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

Masayuki Yamamoto added the comment: On, I got complex type in cloudABI. I see my patch doesn't solve it. https://github.com/NuxiNL/cloudlibc/blob/master/src/include/sys/types.h#L94 https://github.com/NuxiNL/cloudlibc/blob/master/src/include/_/types.h#L209 --

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

Ed Schouten added the comment: CloudABI uses a structure type, which is exactly why I filed this bug report. Instead of speculating about what kind of type existing implementations use, please just focus on the specification to which we're trying to stick: POSIX.

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

Masayuki Yamamoto added the comment: Umm, API seems a design that is passing into function by integer or pointer because the users don't need type detail. I think the implementation of complex type is not realistic. Actually, CouldABI and Cygwin are used pointer, and type detail is hided.

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

Ed Schouten added the comment: It can also be a structure or a union. -- ___ Python tracker ___ ___

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

Masayuki Yamamoto added the comment: Hi, I came from #28656. I have read past discussions, And I've understood the pthread_key_t has possible of either integer or pointer. So I think there is a simple solution that replaces key type to intptr_t. Thus I wrote a patch, but it maybe need

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

Erik Bray added the comment: Ah, I wasn't thinking clearly toward the bottom of my last message. I see now that after a fork, _PyGILState_Reinit calls PyThread_delete_key followed by a new PyThread_create_key--in the current version of my patch that would result in putting the autoTLSkey at

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

Ed Schouten added the comment: I think that PEP 11 also doesn't rule out changes in this area. For example, consider the paragraph starting with the sentence "This policy does not disqualify supporting other platforms indirectly. [...]" Attached is a patch that accomplishes the same with a

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

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

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

STINNER Victor added the comment: -1 for your change issue25658-1.patch. It adds a O(n) slowdown to PyThread_set_key_value() whereas the performance of this function matters. Moreover, the code currently works fine on Linux, I fail to see why we should make Python slower to support a new

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

Changes by Erik Bray : -- stage: -> patch review ___ Python tracker ___ ___

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

Changes by Erik Bray : -- keywords: +patch Added file: https://bugs.python.org/file44269/issue25658-1.patch ___ Python tracker

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

Erik Bray added the comment: Here's a first stab at a patch for this. A linked list is maintained which maps pthread_key_t instances to an int "key_id" that is used for the PyThread API. Each function needs to map a given key_id to the actual pthread_key_t. This adds a little bit of

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

Changes by R. David Murray : -- hgrepos: -352 ___ Python tracker ___ ___

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

Erik Bray added the comment: FWIW I've created an initial patch for this. Seems to work fine, but it's a bit of a mess and I have a few small implementation concerns. I'll try to get it cleaned up sometime in the next few days and I'll post it. --

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

Erik Bray added the comment: The good news about this (in the pthread case) is that it does not need to be seen as some workaround for unusual platforms, but rather making the existing code more POSIX-compliant (and hence correct). The Win32 side I'm a little less worried about because the

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

R. David Murray added the comment: I'd say that sounds reasonable, but most likely it will only be someone working with one of the impacted platforms who will have the motivation to come up with a patch. Especially since neither of the impacted platforms is current formally supported

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

Erik Bray added the comment: (Of course, maintaining such a list might take some care, but only when creating and deleting keys--it wouldn't add any overhead to using them to get/set values.) -- ___ Python tracker

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

Erik Bray added the comment: I'm not really sure what "long" has to do with it... The problem is that the PyThread API uses ints to represent TLS keys, and has for about as long as it's existed (maybe what you meant by "long"). But when support for native TLS was added (#9786 for pthread,

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

STINNER Victor added the comment: What do you suggest? Python C code bases uses "long" everywhere. -- nosy: +haypo ___ Python tracker ___

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

Erik Bray added the comment: I agree--this has the same problem on Cygwin, where pthread_key_t is not just a typedef'd integer (in fact it's a pointer to an instance of a class). Anyways as Ed wrote above POSIX says this is supposed to be an opaque type and there's no reason to assume it's an

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

New submission from Ed Schouten: While trying to port Python over to a new platform (CloudABI), I noticed a couple of compiler errors in PyThread_create_key(), PyThread_delete_key(), PyThread_delete_key_value() and PyThread_set_key_value() caused by fact that pthread_key_t is converted to an