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

Immortalize Py_EMPTY_KEYS #104252

Closed
ericsnowcurrently opened this issue May 6, 2023 · 0 comments
Closed

Immortalize Py_EMPTY_KEYS #104252

ericsnowcurrently opened this issue May 6, 2023 · 0 comments
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented May 6, 2023 •
edited by bedevere-bot
Loading

The every dict has a keys "object", of type PyDictKeysObject. While it isn't actually a Python object, it does have a refcount, which is used to know when to free it. PyDictKeysObject (and the helpers, dictkeys_incref() and dictkeys_decref()) was not updated to be immortal when the other singletons were. When it comes to interpreter isolation, that's a problem for empty dicts.

Every empty dict shares a global, statically allocated singleton for its keys: Py_EMPTY_KEYS (AKA static PyDictKeysObject empty_keys_struct). This singleton is defined and used internally in dictobject.c, so we don't have the same ABI compatibility concerns that we have with object ref counts generally,

One way or another, we need to isolate Py_EMPTY_KEYS. Otherwise we end up with races on the refcount.

cc @eduardo-elizondo @markshannon

Possible solutions:

  1. update the code in dictobject.c to make Py_EMPTY_KEYS immortal
  2. move Py_EMPTY_KEYS to PyInterpreterState

The first one seems simpler.

Linked PRs
@ericsnowcurrently ericsnowcurrently added type-feature A feature request or enhancement interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 bugs and security fixes labels May 6, 2023
@bedevere-bot bedevere-bot mentioned this issue May 6, 2023
Merged
ericsnowcurrently added a commit that referenced this issue May 10, 2023
@ericsnowcurrently
gh-104252: Immortalize Py_EMPTY_KEYS (gh-104253)
b8f7ab5 
This was missed in gh-19474.  It matters for with a per-interpreter GIL since PyDictKeysObject.dk_refcnt breaks isolation and leads to races.
carljm added a commit to carljm/cpython that referenced this issue May 10, 2023
@carljm
Merge branch 'main' into fixloadsuperspec
5976df9 
* main:
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  pythongh-103247: clear the module cache in a test in test_importlib/extensions/test_loader.py (pythonGH-104226)
  pythongh-103848: Adds checks to ensure that bracketed hosts found by urlsplit are of IPv6 or IPvFuture format (python#103849)
  pythongh-74895: adjust tests to work on Solaris (python#104326)
  pythongh-101819: Refactor _io in preparation for module isolation (python#104334)
  pythongh-90953: Don't use deprecated AST nodes in clinic.py (python#104322)
  pythongh-102327: Extend docs for "url" and "headers" parameters to HTTPConnection.request()
  pythongh-104328: Fix typo in ``typing.Generic`` multiple inheritance error message (python#104335)
carljm added a commit to carljm/cpython that referenced this issue May 11, 2023
@carljm
Merge branch 'main' into supermon
19b3d9e 
* main: (27 commits)
  pythongh-87849: fix SEND specialization family definition (pythonGH-104268)
  pythongh-101819: Adapt _io.IOBase.seek and _io.IOBase.truncate to Argument Clinic (python#104384)
  pythongh-101819: Adapt _io._Buffered* methods to Argument Clinic (python#104367)
  pythongh-101819: Refactor `_io` futher in preparation for module isolation (python#104369)
  pythongh-101819: Adapt _io.TextIOBase methods to Argument Clinic (python#104383)
  pythongh-101117: Improve accuracy of sqlite3.Cursor.rowcount docs (python#104287)
  pythonGH-92184: Convert os.altsep to '/' in filenames when creating ZipInfo objects (python#92185)
  pythongh-104357: fix inlined comprehensions that close over iteration var (python#104368)
  pythonGH-90208: Suppress OSError exceptions from `pathlib.Path.glob()` (pythonGH-104141)
  pythonGH-102181: Improve specialization stats for SEND (pythonGH-102182)
  pythongh-103000: Optimise `dataclasses.asdict` for the common case (python#104364)
  pythongh-103538: Remove unused TK_AQUA code (pythonGH-103539)
  pythonGH-87695: Fix OSError from `pathlib.Path.glob()` (pythonGH-104292)
  pythongh-104263: Rely on Py_NAN and introduce Py_INFINITY (pythonGH-104202)
  pythongh-104010: Separate and improve docs for `typing.get_origin` and `typing.get_args` (python#104013)
  pythongh-101819: Adapt _io._BufferedIOBase_Type methods to Argument Clinic (python#104355)
  pythongh-103960: Dark mode: invert image brightness (python#103983)
  pythongh-104252: Immortalize Py_EMPTY_KEYS (pythongh-104253)
  pythongh-101819: Clean up _io windows console io after pythongh-104197 (python#104354)
  pythongh-101819: Harden _io init (python#104352)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ericsnowcurrently