Issue 28991: Fix obscure lru_cache reentrancy bug (original) (raw)

This issue has been migrated to GitHub: https://github.com/python/cpython/issues/73177

classification

Title: Fix obscure lru_cache reentrancy bug
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.5

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-12-16 18:59 by rhettinger, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix_lru_len_reentrancy.diff rhettinger,2016-12-16 18:59 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft,2017-03-31 16:36
Messages (6)
msg283426 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-12-16 18:59
Call __len__ directly inside the LRU cache code rather than using len(). This helps further encapsulate the cache internals making it less dependent on parts of the environment that are subject to change.
msg283427 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 19:10
Wrapping len() in an lru_cache is bad idea, because this brakes len() of lists, dicts and other non-hashable collections. Should we support this doubtful case?
msg283438 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-12-16 21:12
It is effortless to make this change and is consistent with all the other efforts taken to to protect against reentrancy, so yes, I think it should be done. It isn't so much that I want this to be a use case, I would just like the implementation to be as sound as reasonably possible so that we don't find the LRU cache to be at heart of some future hard-to-find bug in a complex application. How does the patch look to you?
msg283444 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 21:38
> How does the patch look to you? It looks ugly, but technically correct. Push it if you like.
msg283445 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-16 22:01
New changeset c23f8614151d by Raymond Hettinger in branch '3.5': Issue #28991: Fix obscure reentrancy bug in functools.lru_cache(). https://hg.python.org/cpython/rev/c23f8614151d
msg283447 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-12-16 23:01
New changeset 5ec5bfcf0089 by Raymond Hettinger in branch 'default': Issue #28991: Address comment that the __len__ call looked unattractive https://hg.python.org/cpython/rev/5ec5bfcf0089
History
Date User Action Args
2022-04-11 14:58:40 admin set github: 73177
2017-03-31 16:36:08 dstufft set pull_requests: + <pull%5Frequest834>
2016-12-16 23:01:04 python-dev set messages: +
2016-12-16 22:16:09 rhettinger set status: open -> closedresolution: fixed
2016-12-16 22:01:29 python-dev set nosy: + python-devmessages: +
2016-12-16 21:38:19 serhiy.storchaka set assignee: serhiy.storchaka -> rhettingermessages: + stage: patch review -> commit review
2016-12-16 21:12:18 rhettinger set messages: +
2016-12-16 19:10:44 serhiy.storchaka set messages: +
2016-12-16 18:59:17 rhettinger create