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

locking behavior removed from functools.cached_property #172

Closed
carljm opened this issue Feb 28, 2023 · 1 comment
Closed

locking behavior removed from functools.cached_property #172

carljm opened this issue Feb 28, 2023 · 1 comment

Comments

@carljm
Copy link
Member

carljm commented Feb 28, 2023

Hello!

Since Python 3.8 when it was introduced, functools.cached_property has had a class-wide writer lock, which provides thread-safety but (being class-wide) can cause pathological lock contention, causing many users to consider functools.cached_property un-usable and maintain their own lock-free versions instead. python/cpython#87634 was opened almost two years ago to report this problem.

A few weeks ago, I created https://discuss.python.org/t/finding-a-path-forward-for-functools-cached-property/23757 to discuss possible paths forward. Most core devs in that thread favored simply removing the locking behavior; a few preferred finding a way to fix it to be per-instance. (No PR for the latter has been put forward so far.) I opened a PR (python/cpython#101890) for the remove-locking option, to prompt discussion and help the issue move forward. I didn't expect the PR to be merged as-is without consensus or an SC ruling, but it was (which is my responsibility; I didn't mark it otherwise.) But given that there wasn't full consensus of core devs, and the issue has to do with backward-compatibility commitments, it seems best practice to get an SC opinion on this.

Possible rulings (there may be others!): (a) go ahead with the status quo, locking will be removed in 3.12 with a note in What's New, (b) yes, we should remove locking, but we should warn about it in What's New one or two versions in advance, and not actually change the behavior until 3.13 or 3.14 (meaning it will also remain unusable, for some users, for longer), or (c) we shouldn't remove the locking at all, we should wait for a PR to hopefully appear to fix the locking to be per-instance without creating too much overhead, and in the meantime let it remain unusable.

If the SC prefers (b) or (c), I will make sure my PR is reverted (at least for now, in the case of (b)).

@brettcannon
Copy link
Member

After talking it over, and we suggest going with option (a); remove it in 3.12 and mention it in What's New. Because it has never quite functioned properly, we view this as a bugfix that shouldn't be backported. We also realized that there's no good way to warn users about the upcoming change while making it easy to be backwards-compatible to Python 3.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants