| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Yeah, this feels like a really big hack.
But, won't this still initialize the types concurrently anyway? I don't think that's thread-safe, even if the main interpreter sets some of the initial values. AFAICT, the current assumption is that only one interpreter can initialize a static type at a time (because the type lock is per-interpreter, not runtime-wide).
Sorry, something went wrong.
I left a comment in your PR: #136583 (comment)
The runtime state has the initial values as common type data. OTOH, non-initial values such as tp_dict are held by each interpreter state. So, once the main interpreter does initial initialization, subinterpreters will focus on their interpreter state with their type lock. Also, I saw #129817 being closed as completed. What concern remains? |
Sorry, something went wrong.
I think that issue was only for tp_flags or tp_versions_used. #129824 is still open. Just looking at the code for init_static_type (or more importantly, type_ready), it doesn't look thread-safe for all interpreters. |
Sorry, something went wrong.
|
According to #129824, the managed static builtin types have the issue despite the fact that the main interpreter initializes them first. Then, the managed static extension types also have the same issue that cannot be fixed by this PR and yours. The same will go for type_ready() if it has issues. |
Sorry, something went wrong.
|
Ugh, I thought that interpreter initialization held the runtime lock. I'll investigate further later this week. We can probably avoid changes to _datetime by just making the static type initialization thread-safe with a big lock or something like that. That said, I do still think _datetime should be a static module regardless of any static type changes; static types should be in static modules. I'll open a DPO thread at some point. |
Sorry, something went wrong.
The managed static extension type has been introduced just to work around the issue of the capsule C API, so you need to mention there why you cannot resolve the issue first. |
Sorry, something went wrong.
|
An alternative could be to immortalize some heap-types and expose them in the capsule. |
Sorry, something went wrong.
Sounds promising, changing the PyDateTimeAPI global variable to something without exposing internals. |
Sorry, something went wrong.
|
But a heaptype usually holds a corresponding module, which needs to be updated. Or, perhaps immortalize the module as well? |
Sorry, something went wrong.
|
I haven't fully fleshed out the idea yet. I'm thinking we could make PyDateTimeAPI thread-local and then make it aware of the thread state somehow. |
Sorry, something went wrong.
|
Closing, as #136583 seems to be backportable. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.