Skip to content

gh-160496: Clear inline values with atomic release stores - #160510

Open
mpage wants to merge 2 commits into
python:mainfrom
mpage:gh-160496-dict-race
Open

mpage wants to merge 2 commits into
python:mainfrom
mpage:gh-160496-dict-race

Conversation

@mpage

@mpage mpage commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Py_CLEAR writes NULL non-atomically, racing with the acquire load in _PyObject_TryGetInstanceAttribute. Invalidating the inline values does not exclude lookups already in flight.

Authored with AI.

Py_CLEAR writes NULL non-atomically, racing with the acquire load in
_PyObject_TryGetInstanceAttribute. Invalidating the inline values does not
exclude lookups already in flight. Use a release store before decref, preserving
the clear-before-decref ordering.
Comment thread Objects/dictobject.c
for (Py_ssize_t i = 0; i < values->capacity; i++) {
Py_CLEAR(values->values[i]);
PyObject *value = values->values[i];
FT_ATOMIC_STORE_PTR_RELEASE(values->values[i], NULL);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth a comment about what the conflicting reads are?

@nascheme

Copy link
Copy Markdown
Member

Maybe more explicit about where the load is, e.g.

  // Pair with the acquire load in _PyObject_TryGetInstanceAttribute(),
  // which may read this slot after passing the initial validity check.
  FT_ATOMIC_STORE_PTR_RELEASE(values->values[i], NULL);

Or, do you not want to explicitly name the function?

@mpage

mpage commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

Maybe more explicit about where the load is, e.g.

  // Pair with the acquire load in _PyObject_TryGetInstanceAttribute(),
  // which may read this slot after passing the initial validity check.
  FT_ATOMIC_STORE_PTR_RELEASE(values->values[i], NULL);

Or, do you not want to explicitly name the function?

I was explicitly not naming the function to avoid the situation where the comment gets out of date due to refactoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants