GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-101696: invalidate type version tag in `_PyStaticType_Dealloc ` by kumaraditya303 · Pull Request #101697 · python/cpython

Conversation

@kumaraditya303

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Issue: _PyStaticType_Dealloc does not invalidate type version tag #101696

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@kumaraditya303
invalidate cache counter
b5dc0dd
@kumaraditya303
kumaraditya303 requested a review from markshannon as a code owner February 8, 2023 17:11
@bedevere-bot bedevere-bot added the awaiting core review label Feb 8, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 8, 2023
_PyStaticType_Dealloc does not invalidate type version tag #101696
Closed
@blurb-it
📜🤖 Added by blurb_it.
60b1ec0
@kumaraditya303
kumaraditya303 requested a review from erlend-aasland February 8, 2023 17:15
@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
erlend-aasland
erlend-aasland reviewed Feb 8, 2023
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2023-02-08-17-13-31.gh-issue-101696.seJhTt.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
Update Misc/NEWS.d/next/Core and Builtins/2023-02-08-17-13-31.gh-issu…
c074ee1 
…e-101696.seJhTt.rst
erlend-aasland
erlend-aasland approved these changes Feb 8, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 8, 2023
@kumaraditya303
kumaraditya303 merged commit d9de079 into python:main Feb 8, 2023
@miss-islington

miss-islington commented Feb 8, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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

miss-islington commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Feb 9, 2023

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned kumaraditya303 Feb 9, 2023
@bedevere-bot

bedevere-bot commented Feb 9, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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

markshannon commented Feb 9, 2023

Copy link
Copy Markdown
Member

This makes no sense to me.

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

See #101603
ambv reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ambv ambv mentioned this pull request Feb 9, 2023
[3.11] GH-101696: invalidate type version tag in _PyStaticType_Dealloc (GH-101697) #101722
Merged
@kumaraditya303

kumaraditya303 commented Feb 9, 2023

Copy link
Copy Markdown
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.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon

markshannon commented Feb 9, 2023

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Feb 9, 2023

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland erlend-aasland mentioned this pull request Feb 9, 2023
gh-101696: Make sure immutable types have a valid version tag #101742
Closed
@ericsnowcurrently

ericsnowcurrently commented Feb 9, 2023

Copy link
Copy Markdown
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.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@kumaraditya303 @miss-islington @bedevere-bot @markshannon @erlend-aasland @ericsnowcurrently
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.