Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-101696: invalidate type version tag in _PyStaticType_Dealloc #101697

Merged
merged 3 commits into from
Feb 8, 2023
Merged

GH-101696: invalidate type version tag in _PyStaticType_Dealloc #101697

merged 3 commits into from
Feb 8, 2023

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Feb 8, 2023 •
edited by bedevere-bot
Loading

@bedevere-bot bedevere-bot mentioned this pull request Feb 8, 2023
Closed
@kumaraditya303 kumaraditya303 added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) needs backport to 3.11 only security fixes labels Feb 8, 2023
@kumaraditya303 kumaraditya303 merged commit d9de079 into python:main Feb 8, 2023
@miss-islington
Copy link
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@kumaraditya303 kumaraditya303 deleted the versiontag branch February 8, 2023 18:02
@erlend-aasland erlend-aasland added needs backport to 3.11 only security fixes and removed needs backport to 3.11 only security fixes labels Feb 9, 2023
@miss-islington
Copy link
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @kumaraditya303, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d9de0792482d2ded364b0c7d2867b97a5da41b12 3.11

@bedevere-bot
Copy link

GH-101722 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Feb 9, 2023
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Feb 9, 2023
@kumaraditya303 @erlend-aasland
[3.11] pythonGH-101696: invalidate type version tag in `_PyStaticType…
5f83520 
…_Dealloc` (pythonGH-101697).

(cherry picked from commit d9de079)

Co-authored-by: Kumar Aditya <[email protected]>
@markshannon
Copy link
Member

This makes no sense to me.

Static types should be immutable; their version number should never change.

See #101603

@ambv ambv mentioned this pull request Feb 9, 2023
Merged
@kumaraditya303
Copy link
Contributor Author

Static types should be immutable; their version number should never change.

It is still the case but when the interpreter is initialized multiple times then the type still has the same old version tag and can cause assertion failures. Note that the type cache is per interp now and is cleared on every cycle. See https://github.com/python/cpython/actions/runs/4112993949/jobs/7098592004 as an example.

@markshannon
Copy link
Member

I don't think that this PR fixes the root problem, although it almost certainly no worse than what we had before.

What assertions fails? "Can cause assertion failures" doesn't tell me anything.

When you say "the interpreter" how does this handle sub-interpreters?
The problems are that heap and static types are sharing the same versions, and that versions are shared across interpreters.

@ericsnowcurrently might have thoughts on this.

@erlend-aasland
Copy link
Contributor

What assertions fails? "Can cause assertion failures" doesn't tell me anything.

See the GH Action run that Kumar linked to for an example. I'll quote it for you:

Objects/typeobject.c:4174: _PyType_Lookup: Assertion `_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)' failed.

This is from test_finalize_structseq in test_embed.

@erlend-aasland erlend-aasland mentioned this pull request Feb 9, 2023
Closed
@ericsnowcurrently
Copy link
Member

When you say "the interpreter" how does this handle sub-interpreters?
The problems are that heap and static types are sharing the same versions, and that versions are shared across interpreters.

As @kumaraditya303 mentioned in gh-101696, subinterpreters will be dealt with via gh-95795.

carljm added a commit to carljm/cpython that referenced this pull request Feb 9, 2023
@carljm
Merge branch 'main' into inlinecomp2
be3becc 
* main: (82 commits)
  pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723)
  pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736)
  no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731)
  pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730)
  pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679)
  pythongh-85984: Remove legacy Lib/pty.py code. (python#92365)
  pythongh-98831: Use opcode metadata for stack_effect() (python#101704)
  pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719)
  pythongh-101283: Fix use of unbound variable (pythonGH-101712)
  pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286)
  pythongh-101277: Port more itertools static types to heap types (python#101304)
  pythongh-98831: Modernize CALL and family (python#101508)
  pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697)
  pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329)
  pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672)
  pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324)
  pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615)
  pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934)
  pythonGH-101578: Normalize the current exception (pythonGH-101607)
  pythongh-47937: Note that Popen attributes are read-only (python#93070)
  ...
kumaraditya303 added a commit that referenced this pull request Feb 11, 2023
@erlend-aasland @kumaraditya303
[3.11] GH-101696: invalidate type version tag in `_PyStaticType_Deall…
c5c1238 
…oc` (GH-101697) (#101722)

[3.11] GH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (GH-101697).
(cherry picked from commit d9de079)

Co-authored-by: Kumar Aditya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

@kumaraditya303 kumaraditya303

Labels
awaiting merge interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kumaraditya303 @miss-islington @bedevere-bot @markshannon @erlend-aasland @ericsnowcurrently