Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-117953: Track Extra Details in Global Extensions Cache#118532

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrentlyericsnowcurrently commentedMay 3, 2024
edited by bedevere-appbot
Loading

We have only been tracking each module'sPyModuleDef. However, there are some problems with that. For example, in some cases we load single-phase init extension modules fromdef->m_base.m_init ordef->m_base.m_copy, but if multiple modules share a def then we can end up with unexpected behavior.

With this change, we track the following:

  • PyModuleDef (same as before)
  • for some modules, its init function or a copy of its__dict__, but specific to that module
  • whether it is a builtin/core module or a "dynamic" extension
  • the interpreter (ID) that owns the cached__dict__ (only if cached)

This also makes it easier to remember the module's kind (e.g. single-phase init) and if loading it previously failed, which I'm doing in another PR.

@ericsnowcurrentlyericsnowcurrentlyenabled auto-merge (squash)May 4, 2024 21:04
@ericsnowcurrentlyericsnowcurrently merged commit291cfa4 intopython:mainMay 4, 2024
34 checks passed
@ericsnowcurrentlyericsnowcurrently deleted the extensions-cache-better-tracking branchMay 5, 2024 02:43
SonicField pushed a commit to SonicField/cpython that referenced this pull requestMay 8, 2024
…ongh-118532)We have only been tracking each module's PyModuleDef.  However, there are some problems with that.  For example, in some cases we load single-phase init extension modules from def->m_base.m_init or def->m_base.m_copy, but if multiple modules share a def then we can end up with unexpected behavior.With this change, we track the following:* PyModuleDef (same as before)* for some modules, its init function or a copy of its __dict__, but specific to that module* whether it is a builtin/core module or a "dynamic" extension* the interpreter (ID) that owns the cached __dict__ (only if cached)This also makes it easier to remember the module's kind (e.g. single-phase init) and if loading it previously failed, which I'm doing separately.
@hugovk
Copy link
Member

We're gettingpython: Python/import.c:460: _get_module_index_from_def: Assertion `index > 0' failed. when trying toimport ujson on 3.13, and git bisect points to this PR.

Seeultrajson/ultrajson#629.

Does it sound like a CPython or ujson problem?

ericsnowcurrently reacted with thumbs up emoji

@ericsnowcurrently
Copy link
MemberAuthor

Sorry for the delay. PyCon was a bit distracting. :) I'll take a look.

hugovk reacted with thumbs up emoji

ericsnowcurrently added a commit that referenced this pull requestMay 25, 2024
The assertion was added ingh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 27, 2024
…ongh-119561)The assertion was added inpythongh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.(cherry picked from commit0c5ebe1)Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
ericsnowcurrently added a commit that referenced this pull requestMay 27, 2024
…119561) (gh-119632)The assertion was added ingh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.(cherry picked from commit0c5ebe1)Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
estyxx pushed a commit to estyxx/cpython that referenced this pull requestJul 17, 2024
…ongh-119561)The assertion was added inpythongh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ericsnowcurrently@hugovk

[8]ページ先頭

©2009-2025 Movatter.jp