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.

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

Dictionary watchers deliver added event before it's guaranteed to be successful leading to possible incosistent state #122208

Closed
DinoV opened this issue Jul 23, 2024 · 1 comment
Labels
type-bug An unexpected behavior, bug, or error

Comments

@DinoV
Copy link
Contributor

DinoV commented Jul 23, 2024

Bug report

Bug description:

Currently dictionary watchers deliver the PyDict_EVENT_ADDED event before they have done everything that is necessary to ensure success. If the dictionary requires a resize for insertion to succeed (or a new keys object needs to be allocated from an empty dict) and that fails then the dictionary watcher will have inconsistent state to what's stored in the dictionary. Watchers should only receive the event when the dictionary implementation is guaranteed that it will succeed.

The event can still be delivered before the insertion happens meaning there's no visible change in behavior except for the OOM case.

CPython versions tested on:

3.12, CPython main branch

Operating systems tested on:

Linux

Linked PRs

@DinoV DinoV added the type-bug An unexpected behavior, bug, or error label Jul 23, 2024
DinoV added a commit that referenced this issue Jul 24, 2024
…2207)

Don't delivery PyDict_EVENT_ADDED until it can't fail
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this issue Jul 24, 2024
Summary:
Back port of: python/cpython#122207 fixing python/cpython#122208

The current dictionary watchers implementation delivers the added event before it checks to see if we need to re-size the dictionary. This resize can fail so the value isn't added, and then the tracker is out of sync with the true state of the dictionary.

This moves the delivery of the event to after any necessary allocations have happened.

Reviewed By: jbower-fb

Differential Revision: D60182094

fbshipit-source-id: f34940e98ce1caadeee364f9d126d35839661961
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this issue Jul 24, 2024
Summary:
Back port of: python/cpython#122207 fixing python/cpython#122208

The current dictionary watchers implementation delivers the added event before it checks to see if we need to re-size the dictionary. This resize can fail so the value isn't added, and then the tracker is out of sync with the true state of the dictionary.

This moves the delivery of the event to after any necessary allocations have happened.

Reviewed By: jbower-fb

Differential Revision: D60182094

fbshipit-source-id: f34940e98ce1caadeee364f9d126d35839661961
nohlson pushed a commit to nohlson/cpython that referenced this issue Jul 24, 2024
nohlson pushed a commit to nohlson/cpython that referenced this issue Jul 24, 2024
DinoV added a commit that referenced this issue Jul 30, 2024
…il (#122326)

Don't delivery PyDict_EVENT_ADDED until it can't fail
DinoV added a commit that referenced this issue Jul 30, 2024
…il (#122327)

Don't delivery PyDict_EVENT_ADDED until it can't fail
@hugovk
Copy link
Member

hugovk commented Aug 8, 2024

Triage: closing as the linked PRs are merged, please re-open if still needed.

@hugovk hugovk closed this as completed Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants