This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

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:40adminsetgithub: 73177
2017-03-31 16:36:08dstufftsetpull_requests: + pull_request834
2016-12-16 23:01:04python-devsetmessages: + msg283447
2016-12-16 22:16:09rhettingersetstatus: open -> closed
resolution: fixed
2016-12-16 22:01:29python-devsetnosy: + python-dev
messages: + msg283445
2016-12-16 21:38:19serhiy.storchakasetassignee: serhiy.storchaka -> rhettinger
messages: + msg283444
stage: patch review -> commit review
2016-12-16 21:12:18rhettingersetmessages: + msg283438
2016-12-16 19:10:44serhiy.storchakasetmessages: + msg283427
2016-12-16 18:59:17rhettingercreate