From 9f073b94a24715f5e581f7738c012358a18afffd Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Fri, 18 Jan 2019 17:55:17 -0800
Subject: [PATCH 01/30] Communicate that the pointer updates doen't change
 link->prev and link->next references. This saves an unnecessary duplicate
 lookup.

Clang assembly before:

    movq    16(%rax), %rcx      # link->prev
    movq    24(%rax), %rdx      # link->next
    movq    %rdx, 24(%rcx)      # link->prev->next = link->next;
    movq    24(%rax), %rdx      # duplicate fetch of link->next
    movq    %rcx, 16(%rdx)      # link->next->prev = link->prev;

Clang assembly after:

    movq    16(%rax), %rcx
    movq    24(%rax), %rdx
    movq    %rdx, 24(%rcx)
    movq    %rcx, 16(%rdx)
---
 Modules/_functoolsmodule.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 0fb4847af9c371..8cbd79ceaf1a9d 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -837,8 +837,10 @@ infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd
 static void
 lru_cache_extricate_link(lru_list_elem *link)
 {
-    link->prev->next = link->next;
-    link->next->prev = link->prev;
+    lru_list_elem *link_prev = link->prev;
+    lru_list_elem *link_next = link->next;
+    link_prev->next = link->next;
+    link_next->prev = link->prev;
 }
 
 static void

From 42ab33140a0495862b7e2212392c8ec7f2c52213 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 00:02:47 -0800
Subject: [PATCH 02/30] Save space for scalar keys

---
 Modules/_functoolsmodule.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 8cbd79ceaf1a9d..ab07fd03b53f67 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -733,6 +733,15 @@ lru_cache_make_key(PyObject *args, PyObject *kwds, int typed)
 
     /* short path, key will match args anyway, which is a tuple */
     if (!typed && !kwds) {
+        if (PyTuple_GET_SIZE(args) == 1) {
+            key = PyTuple_GET_ITEM(args, 0);
+            if (!PySequence_Check(key)) {
+                /* For scalar keys, save space and
+                   drop the enclosing args tuple  */
+                Py_INCREF(key);
+                return key;
+            }
+        }
         Py_INCREF(args);
         return args;
     }

From fb148f3b4142841ba6b5cf7ba81e06417c15f6ca Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 12:48:41 -0800
Subject: [PATCH 03/30] Fix diction

---
 Modules/_functoolsmodule.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index ab07fd03b53f67..f70ccae052d7b9 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -844,7 +844,7 @@ infinite_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwd
 }
 
 static void
-lru_cache_extricate_link(lru_list_elem *link)
+lru_cache_extract_link(lru_list_elem *link)
 {
     lru_list_elem *link_prev = link->prev;
     lru_list_elem *link_next = link->next;
@@ -879,7 +879,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
     }
     link  = (lru_list_elem *)_PyDict_GetItem_KnownHash(self->cache, key, hash);
     if (link) {
-        lru_cache_extricate_link(link);
+        lru_cache_extract_link(link);
         lru_cache_append_link(self, link);
         self->hits++;
         result = link->result;
@@ -899,9 +899,9 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
     if (self->full && self->root.next != &self->root) {
         /* Use the oldest item to store the new key and result. */
         PyObject *oldkey, *oldresult, *popresult;
-        /* Extricate the oldest item. */
+        /* Extract the oldest item. */
         link = self->root.next;
-        lru_cache_extricate_link(link);
+        lru_cache_extract_link(link);
         /* Remove it from the cache.
            The cache dict holds one reference to the link,
            and the linked list holds yet one reference to it. */

From c8558d674f82e68be13d27c97663383778253580 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 16:54:00 -0800
Subject: [PATCH 04/30] Fix bug arising in recursive user calls

---
 Lib/test/test_functools.py | 28 ++++++++++++++++++++++++++++
 Modules/_functoolsmodule.c | 30 +++++++++++++++++++++++++-----
 2 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py
index ffbd0fcf2d80f2..7982b61da15400 100644
--- a/Lib/test/test_functools.py
+++ b/Lib/test/test_functools.py
@@ -1233,6 +1233,34 @@ def f(x):
         self.assertEqual(misses, 4)
         self.assertEqual(currsize, 2)
 
+    def test_lru_bug_35780(self):
+        # C version of the lru_cache was not checking to see if
+        # the user function call has already modified the cache
+        # (this arises in recursive calls and in multi-threading).
+        # This cause the cache to have orphan links not referenced
+        # by the cache dictionary.
+        global _once
+
+        _once = True
+
+        @self.module.lru_cache(maxsize=10)
+        def f(x):
+            global _once
+            rv = f'.{x}.'
+            if x == 20 and _once:
+                _once = False
+                rv = f(x)
+            return rv
+
+        # Fill the cache
+        for x in range(15):
+            self.assertEqual(f(x), f'.{x}.')
+        self.assertEqual(f.cache_info().currsize, 10)
+
+        # Make a recursive call and make sure the cache remains full
+        self.assertEqual(f(20), '.20.')
+        self.assertEqual(f.cache_info().currsize, 10)
+
     def test_lru_hash_only_once(self):
         # To protect against weird reentrancy bugs and to improve
         # efficiency when faced with slow __hash__ methods, the
diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index f70ccae052d7b9..20082e162316bb 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -866,7 +866,7 @@ static PyObject *
 bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds)
 {
     lru_list_elem *link;
-    PyObject *key, *result;
+    PyObject *key, *result, *testresult;
     Py_hash_t hash;
 
     key = lru_cache_make_key(args, kwds, self->typed);
@@ -896,9 +896,33 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         Py_DECREF(key);
         return NULL;
     }
+    testresult = _PyDict_GetItem_KnownHash(self->cache, key, hash);
+    if (testresult == NULL) {
+        if (PyErr_Occurred()) {
+            /* This is an unusual case since this same lookup
+               did not previously trigger an error during lookup.
+               Treat it the same as an error in user function
+               and return with the error set. */
+            Py_DECREF(key);
+            return NULL;
+        }
+        /* This is the normal case.  The new key wasn't found before
+           user function call and it is still not there.  So we
+           proceed normally and update the cache with the new result. */
+    }
+    else {
+        /* Getting here means that either an error occurred or that this
+           same key was added to the cache during the PyObject_Call().
+           Since the link update is already done, we need only return
+           the computed result and update the count of misses. */
+        Py_DECREF(key);
+        self->misses++;
+        return result;
+    }
     if (self->full && self->root.next != &self->root) {
         /* Use the oldest item to store the new key and result. */
         PyObject *oldkey, *oldresult, *popresult;
+
         /* Extract the oldest item. */
         link = self->root.next;
         lru_cache_extract_link(link);
@@ -909,10 +933,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                                           link->key, link->hash,
                                           Py_None);
         if (popresult == Py_None) {
-            /* Getting here means that this same key was added to the
-               cache while the lock was released.  Since the link
-               update is already done, we need only return the
-               computed result and update the count of misses. */
             Py_DECREF(popresult);
             Py_DECREF(link);
             Py_DECREF(key);

From 98a638ca64ee8c7df3a256c0fb6fc00468ca870c Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 17:13:08 -0800
Subject: [PATCH 05/30] Add blurb

---
 .../next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst   | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst

diff --git a/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
new file mode 100644
index 00000000000000..9c652484526bfa
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
@@ -0,0 +1,6 @@
+Fix lru_cache() errors arising in recursive, reentrant, or multi-threaded
+code. The error could result in orphan links.
+
+Also apply space saving path for functions with a single positional
+argument.  In this common case, the space overhead of an lru cache entry is
+reduced by almost half.

From 1fecffd195c5aa220ffe5afcc77dbf37b68e8e62 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 17:24:45 -0800
Subject: [PATCH 06/30] Fix bug where in an error case, the old key about to be
 evicted was incorrectly moved to the newest position as if the user had made
 a recent call with this key.   The fix is to restore it the oldest position,
 keeping the LRU invariant where keys are tracked by recency of access.

---
 Modules/_functoolsmodule.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 20082e162316bb..a6f2d8a2a598e4 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -862,6 +862,16 @@ lru_cache_append_link(lru_cache_object *self, lru_list_elem *link)
     link->next = root;
 }
 
+static void
+lru_cache_prepend_link(lru_cache_object *self, lru_list_elem *link)
+{
+    lru_list_elem *root = &self->root;
+    lru_list_elem *first = root->next;
+    first->prev = root->next = link;
+    link->prev = root;
+    link->next = first;
+}
+
 static PyObject *
 bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds)
 {
@@ -938,7 +948,14 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             Py_DECREF(key);
         }
         else if (popresult == NULL) {
-            lru_cache_append_link(self, link);
+            /* An error arose while trying to remove the oldest
+               key (the one being evicted) from the cache.
+               We restore the link to its original position
+               as the oldest link.  Then we allow the error
+               propagate upward; treating it the same as an
+               error arising in the user function.
+            */
+            lru_cache_prepend_link(self, link);
             Py_DECREF(key);
             Py_DECREF(result);
             return NULL;

From 79bfd59edf66826568cb5715295304d5111c9ed4 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 17:39:37 -0800
Subject: [PATCH 07/30] Convert an always true test into an assertion.

Formerly, the code allowed cache to fall into an inconsistent
state.  Now, there are no code paths that have a full cache
but no links.
---
 Modules/_functoolsmodule.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index a6f2d8a2a598e4..e652195796f7ff 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -929,11 +929,12 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         self->misses++;
         return result;
     }
-    if (self->full && self->root.next != &self->root) {
+    if (self->full) {
         /* Use the oldest item to store the new key and result. */
         PyObject *oldkey, *oldresult, *popresult;
 
         /* Extract the oldest item. */
+        assert(self->root.next != &self->root);
         link = self->root.next;
         lru_cache_extract_link(link);
         /* Remove it from the cache.

From 4d664138595eac1db61729745bcf791a904a343c Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 17:56:47 -0800
Subject: [PATCH 08/30] Reset the "full" flag when a link is dropped.  Update
 comments.

---
 Modules/_functoolsmodule.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index e652195796f7ff..426e03f6a600c2 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -930,7 +930,19 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         return result;
     }
     if (self->full) {
-        /* Use the oldest item to store the new key and result. */
+        /* Since the cache is full, we need to evict an old key and add
+           a new key.  Rather than free the old link and allocate a new
+           one, we reuse the link for the new key and result and move it
+           to end of the cache.
+
+           We try to assure all code paths (including errors) leave all
+           of the links in-place (so that the "full" status remains
+           unchanged).  Either the link is successfully updated and
+           moved or it is restored to its old position.
+
+           That said, if an unrecoverable error is found, the orphan
+           link is removed and the cache is no longer marked as full.
+        */
         PyObject *oldkey, *oldresult, *popresult;
 
         /* Extract the oldest item. */
@@ -944,6 +956,14 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                                           link->key, link->hash,
                                           Py_None);
         if (popresult == Py_None) {
+            /* Getting here means that the user function call or
+               another thread has already removed the old key from the
+               dictionary.  This link is now an orpan.  Since we don't
+               want to leave the cache in an inconsistent state, we
+               don't restore the link.  Accordingly, the cache is marked
+               as no longer being full.
+            */
+            self->full = 0;
             Py_DECREF(popresult);
             Py_DECREF(link);
             Py_DECREF(key);

From 8f582dc1b53e864c6956fccd8fed9fd43ab53bdd Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 20:12:34 -0800
Subject: [PATCH 09/30] Fix missing decref.

Also move decrefs to the end of end path to make it easier to
verify that there are no reentrant calls before the cache
invariants have been restored.
---
 Modules/_functoolsmodule.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 426e03f6a600c2..ffbb5cbb7ff6ed 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -914,6 +914,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                Treat it the same as an error in user function
                and return with the error set. */
             Py_DECREF(key);
+            Py_DECREF(result);
             return NULL;
         }
         /* This is the normal case.  The new key wasn't found before
@@ -982,7 +983,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             return NULL;
         }
         else {
-            Py_DECREF(popresult);
             /* Keep a reference to the old key and old result to
                prevent their ref counts from going to zero during the
                update. That will prevent potentially arbitrary object
@@ -996,6 +996,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             link->result = result;
             if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                           hash) < 0) {
+                Py_DECREF(popresult);
                 Py_DECREF(link);
                 Py_DECREF(oldkey);
                 Py_DECREF(oldresult);
@@ -1003,6 +1004,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             }
             lru_cache_append_link(self, link);
             Py_INCREF(result); /* for return */
+            Py_DECREF(popresult);
             Py_DECREF(oldkey);
             Py_DECREF(oldresult);
         }

From 2db2de65e4a8006ae007fe024d0e27171348a478 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sat, 19 Jan 2019 21:43:37 -0800
Subject: [PATCH 10/30] Minor code clarity improvements.  Group link updates
 together. Save hit update for last (as the pure python version does).

---
 Modules/_functoolsmodule.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index ffbb5cbb7ff6ed..94dd8f202cd6a7 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -888,11 +888,11 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         return NULL;
     }
     link  = (lru_list_elem *)_PyDict_GetItem_KnownHash(self->cache, key, hash);
-    if (link) {
+    if (link != NULL) {
         lru_cache_extract_link(link);
         lru_cache_append_link(self, link);
-        self->hits++;
         result = link->result;
+        self->hits++;
         Py_INCREF(result);
         Py_DECREF(key);
         return result;

From f4f969e8ca1a2fc7bd4be554430d22e4f758231a Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sun, 20 Jan 2019 11:45:17 -0800
Subject: [PATCH 11/30] Sync-up C code and pure Python code for scalar argument
 handling.

Limit to just common scalar types to make the space saving
technique easier to reason about (easier to show correctness).
---
 Lib/functools.py           | 2 +-
 Modules/_functoolsmodule.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Lib/functools.py b/Lib/functools.py
index ab7d71e126bd30..2cb419904311e2 100644
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -454,7 +454,7 @@ def __hash__(self):
 
 def _make_key(args, kwds, typed,
              kwd_mark = (object(),),
-             fasttypes = {int, str, frozenset, type(None)},
+             fasttypes = {int, str},
              tuple=tuple, type=type, len=len):
     """Make a cache key from optionally typed positional and keyword arguments
 
diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 94dd8f202cd6a7..1dae59f5b4046f 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -735,9 +735,9 @@ lru_cache_make_key(PyObject *args, PyObject *kwds, int typed)
     if (!typed && !kwds) {
         if (PyTuple_GET_SIZE(args) == 1) {
             key = PyTuple_GET_ITEM(args, 0);
-            if (!PySequence_Check(key)) {
-                /* For scalar keys, save space and
-                   drop the enclosing args tuple  */
+            if (PyUnicode_CheckExact(key) || PyLong_CheckExact(key)) {
+                /* For common scalar keys, save space by
+                   dropping the enclosing args tuple  */
                 Py_INCREF(key);
                 return key;
             }

From 0b8bf71a5accbce55c73151442931a79301b7a2d Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Sun, 20 Jan 2019 13:06:40 -0800
Subject: [PATCH 12/30] Improve clarity and accuracy of new comments

---
 Modules/_functoolsmodule.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 1dae59f5b4046f..f8ef2859ab394e 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -922,10 +922,10 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
            proceed normally and update the cache with the new result. */
     }
     else {
-        /* Getting here means that either an error occurred or that this
-           same key was added to the cache during the PyObject_Call().
-           Since the link update is already done, we need only return
-           the computed result and update the count of misses. */
+        /* Getting here means that this same key was added to the cache
+           during the PyObject_Call().  Since the link update is already
+           done, we need only return the computed result and update the
+           count of misses. */
         Py_DECREF(key);
         self->misses++;
         return result;
@@ -934,7 +934,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         /* Since the cache is full, we need to evict an old key and add
            a new key.  Rather than free the old link and allocate a new
            one, we reuse the link for the new key and result and move it
-           to end of the cache.
+           to front of the cache to mark it as recently used.
 
            We try to assure all code paths (including errors) leave all
            of the links in-place (so that the "full" status remains

From a64e21b3badc475c15148ad552fc6b324a5f289f Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 16:56:10 -0800
Subject: [PATCH 13/30] Cleaner test using a nonlocal variable instead of a
 global

---
 Lib/test/test_functools.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py
index 7982b61da15400..2229bd3008fbf9 100644
--- a/Lib/test/test_functools.py
+++ b/Lib/test/test_functools.py
@@ -1239,16 +1239,15 @@ def test_lru_bug_35780(self):
         # (this arises in recursive calls and in multi-threading).
         # This cause the cache to have orphan links not referenced
         # by the cache dictionary.
-        global _once
 
-        _once = True
+        once = True                 # Modified by f(x) below
 
         @self.module.lru_cache(maxsize=10)
         def f(x):
-            global _once
+            nonlocal once
             rv = f'.{x}.'
-            if x == 20 and _once:
-                _once = False
+            if x == 20 and once:
+                once = False
                 rv = f(x)
             return rv
 

From a3000478152c204770bc2ac03567a8f173a8b6f4 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 18:46:38 -0800
Subject: [PATCH 14/30] Restore cache invariants on a path where an error is
 raised.

---
 Modules/_functoolsmodule.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index f8ef2859ab394e..abac7b29d97e0e 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -996,8 +996,11 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             link->result = result;
             if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                           hash) < 0) {
+                /* Somehow the cache dict update failed.
+                   Restore the old link and let the error
+                   propagate upward.  */
+                lru_cache_prepend_link(self, link);
                 Py_DECREF(popresult);
-                Py_DECREF(link);
                 Py_DECREF(oldkey);
                 Py_DECREF(oldresult);
                 return NULL;

From 84811b5fefccb739d7d519f7406287213b1be997 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 19:15:11 -0800
Subject: [PATCH 15/30] Add general notes on possible sources of reentrancy

---
 Modules/_functoolsmodule.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index abac7b29d97e0e..42e707c0502a01 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -872,6 +872,32 @@ lru_cache_prepend_link(lru_cache_object *self, lru_list_elem *link)
     link->next = first;
 }
 
+/* General note on reentrancy:
+
+   There are four dictionary calls in the bounded_lru_cache_wrapper():
+   1) The initial check for a cache match.  2) The post user-function
+   check for a cache match.  3) The deletion of the oldest entry.
+   4) The addition of the newest entry.
+
+   In all four calls, we have a known hash which lets use avoid a call
+   to __hash__().  That leaves on __eq__ as a possible source of a
+   reentrant call.
+
+   The __eq__ method call is always made for a cache hit (dict access #1).
+   Accordingly, we have to be sure that no modifications to the cache
+   state have happened before this call.
+
+   The __eq__ method call is never made for the deletion (dict access #3)
+   because it is an identity match.
+
+   For the other two accesses (#2 and #4), calls to __eq__ only occur
+   when some other entry happens to have an exactly matching hash (all
+   64-bits).  Though rare, this can happen, so we have to make sure to
+   either call it at the top of its code path before any cache
+   state modifications (dict access #2) or be prepared to restore
+   invariants at the end of the code path (dict access #4).
+ */
+
 static PyObject *
 bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds)
 {

From cc50597ecc03ed0f060d931349458bb49924ee41 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 19:28:06 -0800
Subject: [PATCH 16/30] We can restore the old link, so need to make as no
 longer full.

---
 Modules/_functoolsmodule.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 42e707c0502a01..2ed6745e4b0289 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -1023,10 +1023,12 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                           hash) < 0) {
                 /* Somehow the cache dict update failed.
-                   Restore the old link and let the error
-                   propagate upward.  */
-                lru_cache_prepend_link(self, link);
+                   We no longer can restore the old link,
+                   so mark the cache as not being full
+                   and let the error propagate upward.  */
+                self->full = 0;
                 Py_DECREF(popresult);
+                Py_DECREF(link);
                 Py_DECREF(oldkey);
                 Py_DECREF(oldresult);
                 return NULL;

From 304c104702f56550f234fafecd7851b55bb3a3b0 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 23:20:42 -0500
Subject: [PATCH 17/30] Mention the decref policy in the general notes about
 reentrancy

---
 Modules/_functoolsmodule.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 2ed6745e4b0289..1131880ca40331 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -896,6 +896,11 @@ lru_cache_prepend_link(lru_cache_object *self, lru_list_elem *link)
    either call it at the top of its code path before any cache
    state modifications (dict access #2) or be prepared to restore
    invariants at the end of the code path (dict access #4).
+
+   Another possible source of reentrancy is a decref which can trigger
+   arbitrary code execution.  To make the code easier to reason about,
+   the decrefs are deferred to the end of the each possible code path
+   so that we know the cache is a consistent state.
  */
 
 static PyObject *

From 7ae7208b0b86601d037f961a7847b5d5b45ac77d Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Mon, 21 Jan 2019 23:28:29 -0500
Subject: [PATCH 18/30] Fix grammar in comment block

---
 Modules/_functoolsmodule.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 1131880ca40331..e155da6524a77e 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -880,12 +880,12 @@ lru_cache_prepend_link(lru_cache_object *self, lru_list_elem *link)
    4) The addition of the newest entry.
 
    In all four calls, we have a known hash which lets use avoid a call
-   to __hash__().  That leaves on __eq__ as a possible source of a
+   to __hash__().  That leaves only __eq__ as a possible source of a
    reentrant call.
 
    The __eq__ method call is always made for a cache hit (dict access #1).
-   Accordingly, we have to be sure that no modifications to the cache
-   state have happened before this call.
+   Accordingly, we have make sure not modify the cache state prior to
+   this call.
 
    The __eq__ method call is never made for the deletion (dict access #3)
    because it is an identity match.

From 75ba665ce4f516e13bfe393fbf7ea7902ebddf1f Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Wed, 23 Jan 2019 08:46:14 -0500
Subject: [PATCH 19/30] Fix one more bug for the full status flag.

Since the final setitem is potentially reentrant, we have to reset
the full status prior to the setitem call (since we've already
removed a link and its associated cache dict entry).

After the setitem call, we cannot know whether some other thread
has reset the status, so we cannot just restore it without checking
to make sure the number of dict entries is at maxsize.
---
 Modules/_functoolsmodule.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index e155da6524a77e..0e4009446a9e78 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -1025,13 +1025,13 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             link->hash = hash;
             link->key = key;
             link->result = result;
+            self->full = 0;
             if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                           hash) < 0) {
                 /* Somehow the cache dict update failed.
                    We no longer can restore the old link,
-                   so mark the cache as not being full
+                   so leave the cache marked as not being full
                    and let the error propagate upward.  */
-                self->full = 0;
                 Py_DECREF(popresult);
                 Py_DECREF(link);
                 Py_DECREF(oldkey);
@@ -1039,6 +1039,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                 return NULL;
             }
             lru_cache_append_link(self, link);
+            self->full = (PyDict_GET_SIZE(self->cache) >= self->maxsize);
             Py_INCREF(result); /* for return */
             Py_DECREF(popresult);
             Py_DECREF(oldkey);

From 2fb27c06ee5f05837ece1fa7738d7635b8c7ab84 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Wed, 23 Jan 2019 20:03:25 -0500
Subject: [PATCH 20/30] Fix bug in handling of negative maxsize (should be
 treated as zero).

Negative maxsize was being treated as a cache size of 1
giving an almost 100% miss rate while still incurring
the overhead of cache checking and eviction.

The negative maxsize also showed-up in CacheInfo even though
it was non-sensical to have a negative maxsize.

The negative maxsize also made it into the struct for the C version.
This caused erroneous results for the calculation of the "full" flag.
---
 Lib/functools.py           | 7 +++++--
 Lib/test/test_functools.py | 2 +-
 Modules/_functoolsmodule.c | 3 +++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/Lib/functools.py b/Lib/functools.py
index 2cb419904311e2..b455ecc429952a 100644
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -510,8 +510,11 @@ def lru_cache(maxsize=128, typed=False):
 
     # Early detection of an erroneous call to @lru_cache without any arguments
     # resulting in the inner function being passed to maxsize instead of an
-    # integer or None.
-    if maxsize is not None and not isinstance(maxsize, int):
+    # integer or None.  Negative maxsize is treated as 0.
+    if isinstance(maxsize, int):
+        if maxsize < 0:
+            maxsize = 0
+    elif maxsize is not None:
         raise TypeError('Expected maxsize to be an integer or None')
 
     def decorating_function(user_function):
diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py
index 2229bd3008fbf9..63a9ade54806c5 100644
--- a/Lib/test/test_functools.py
+++ b/Lib/test/test_functools.py
@@ -1356,7 +1356,7 @@ def eq(n):
         for i in (0, 1):
             self.assertEqual([eq(n) for n in range(150)], list(range(150)))
         self.assertEqual(eq.cache_info(),
-            self.module._CacheInfo(hits=0, misses=300, maxsize=-10, currsize=1))
+            self.module._CacheInfo(hits=0, misses=300, maxsize=0, currsize=0))
 
     def test_lru_with_exceptions(self):
         # Verify that user_function exceptions get passed through without
diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 0e4009446a9e78..9b6d4b3e631d27 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -1103,6 +1103,9 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw)
         maxsize = PyNumber_AsSsize_t(maxsize_O, PyExc_OverflowError);
         if (maxsize == -1 && PyErr_Occurred())
             return NULL;
+        if (maxsize < 0) {
+            maxsize = 0;
+        }
         if (maxsize == 0)
             wrapper = uncached_lru_cache_wrapper;
         else

From 3a6973c1884a01487535b36f099b65ce8726397b Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Wed, 23 Jan 2019 20:21:05 -0500
Subject: [PATCH 21/30] Remove the "full" variable.

It is cheaper and more reliable to make on-demand checks for
whether the cache is full than it is to recompute and occasionally
toggle a persistent state variable.
---
 Modules/_functoolsmodule.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 9b6d4b3e631d27..24981fa41d09a8 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -720,7 +720,6 @@ typedef struct lru_cache_object {
     Py_ssize_t misses, hits;
     int typed;
     PyObject *dict;
-    int full;
 } lru_cache_object;
 
 static PyTypeObject lru_cache_type;
@@ -961,19 +960,19 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         self->misses++;
         return result;
     }
-    if (self->full) {
+    assert(self->maxsize > 0);
+    if (PyDict_GET_SIZE(self->cache) >= self->maxsize) {
         /* Since the cache is full, we need to evict an old key and add
            a new key.  Rather than free the old link and allocate a new
            one, we reuse the link for the new key and result and move it
            to front of the cache to mark it as recently used.
 
            We try to assure all code paths (including errors) leave all
-           of the links in-place (so that the "full" status remains
-           unchanged).  Either the link is successfully updated and
-           moved or it is restored to its old position.
-
-           That said, if an unrecoverable error is found, the orphan
-           link is removed and the cache is no longer marked as full.
+           of the links in place.  Either the link is successfully
+           updated and moved or it is restored to its old position.
+           However if an unrecoverable error is found, it doesn't
+           make sense to reinsert the link, so we leave it out
+           and the cache will no longer register as full.
         */
         PyObject *oldkey, *oldresult, *popresult;
 
@@ -992,10 +991,8 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                another thread has already removed the old key from the
                dictionary.  This link is now an orpan.  Since we don't
                want to leave the cache in an inconsistent state, we
-               don't restore the link.  Accordingly, the cache is marked
-               as no longer being full.
+               don't restore the link.
             */
-            self->full = 0;
             Py_DECREF(popresult);
             Py_DECREF(link);
             Py_DECREF(key);
@@ -1025,13 +1022,12 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             link->hash = hash;
             link->key = key;
             link->result = result;
-            self->full = 0;
             if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                           hash) < 0) {
                 /* Somehow the cache dict update failed.
-                   We no longer can restore the old link,
-                   so leave the cache marked as not being full
-                   and let the error propagate upward.  */
+                   We no longer can restore the old link.
+                   Let the error propagate upward and
+                   leave the cache short one link. */
                 Py_DECREF(popresult);
                 Py_DECREF(link);
                 Py_DECREF(oldkey);
@@ -1039,7 +1035,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
                 return NULL;
             }
             lru_cache_append_link(self, link);
-            self->full = (PyDict_GET_SIZE(self->cache) >= self->maxsize);
             Py_INCREF(result); /* for return */
             Py_DECREF(popresult);
             Py_DECREF(oldkey);
@@ -1065,7 +1060,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         }
         lru_cache_append_link(self, link);
         Py_INCREF(result); /* for return */
-        self->full = (PyDict_GET_SIZE(self->cache) >= self->maxsize);
     }
     self->misses++;
     return result;
@@ -1209,7 +1203,6 @@ lru_cache_cache_clear(lru_cache_object *self, PyObject *unused)
 {
     lru_list_elem *list = lru_cache_unlink_list(self);
     self->hits = self->misses = 0;
-    self->full = 0;
     PyDict_Clear(self->cache);
     lru_cache_clear_list(list);
     Py_RETURN_NONE;

From be775c8ebf646ec4c5e86164a2f88a69b4db58e5 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Wed, 23 Jan 2019 22:00:08 -0500
Subject: [PATCH 22/30] Drop unneeded maxsize_O attribute.  Prefer plain
 maxsize as a single source of truth.

---
 Modules/_functoolsmodule.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 24981fa41d09a8..24ab685443faf9 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -712,7 +712,6 @@ typedef PyObject *(*lru_cache_ternaryfunc)(struct lru_cache_object *, PyObject *
 typedef struct lru_cache_object {
     lru_list_elem root;  /* includes PyObject_HEAD */
     Py_ssize_t maxsize;
-    PyObject *maxsize_O;
     PyObject *func;
     lru_cache_ternaryfunc wrapper;
     PyObject *cache;
@@ -1122,8 +1121,6 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw)
     obj->root.prev = &obj->root;
     obj->root.next = &obj->root;
     obj->maxsize = maxsize;
-    Py_INCREF(maxsize_O);
-    obj->maxsize_O = maxsize_O;
     Py_INCREF(func);
     obj->func = func;
     obj->wrapper = wrapper;
@@ -1165,7 +1162,6 @@ lru_cache_dealloc(lru_cache_object *obj)
     PyObject_GC_UnTrack(obj);
 
     list = lru_cache_unlink_list(obj);
-    Py_XDECREF(obj->maxsize_O);
     Py_XDECREF(obj->func);
     Py_XDECREF(obj->cache);
     Py_XDECREF(obj->dict);
@@ -1193,8 +1189,13 @@ lru_cache_descr_get(PyObject *self, PyObject *obj, PyObject *type)
 static PyObject *
 lru_cache_cache_info(lru_cache_object *self, PyObject *unused)
 {
-    return PyObject_CallFunction(self->cache_info_type, "nnOn",
-                                 self->hits, self->misses, self->maxsize_O,
+    if (self->maxsize == -1) {
+        return PyObject_CallFunction(self->cache_info_type, "nnOn",
+                                     self->hits, self->misses, Py_None,
+                                     PyDict_GET_SIZE(self->cache));
+    }
+    return PyObject_CallFunction(self->cache_info_type, "nnnn",
+                                 self->hits, self->misses, self->maxsize,
                                  PyDict_GET_SIZE(self->cache));
 }
 
@@ -1238,7 +1239,6 @@ lru_cache_tp_traverse(lru_cache_object *self, visitproc visit, void *arg)
         Py_VISIT(link->result);
         link = next;
     }
-    Py_VISIT(self->maxsize_O);
     Py_VISIT(self->func);
     Py_VISIT(self->cache);
     Py_VISIT(self->cache_info_type);
@@ -1250,7 +1250,6 @@ static int
 lru_cache_tp_clear(lru_cache_object *self)
 {
     lru_list_elem *list = lru_cache_unlink_list(self);
-    Py_CLEAR(self->maxsize_O);
     Py_CLEAR(self->func);
     Py_CLEAR(self->cache);
     Py_CLEAR(self->cache_info_type);

From 23114ef1681a97ccf25148bb2b840299aabd48b7 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Thu, 24 Jan 2019 00:01:23 -0500
Subject: [PATCH 23/30] Make control clearer by favoring intermediate return
 statements instead of else clauses

---
 Modules/_functoolsmodule.c | 114 ++++++++++++++++++-------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 24ab685443faf9..1b1bf513015221 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -936,21 +936,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         return NULL;
     }
     testresult = _PyDict_GetItem_KnownHash(self->cache, key, hash);
-    if (testresult == NULL) {
-        if (PyErr_Occurred()) {
-            /* This is an unusual case since this same lookup
-               did not previously trigger an error during lookup.
-               Treat it the same as an error in user function
-               and return with the error set. */
-            Py_DECREF(key);
-            Py_DECREF(result);
-            return NULL;
-        }
-        /* This is the normal case.  The new key wasn't found before
-           user function call and it is still not there.  So we
-           proceed normally and update the cache with the new result. */
-    }
-    else {
+    if (testresult != NULL) {
         /* Getting here means that this same key was added to the cache
            during the PyObject_Call().  Since the link update is already
            done, we need only return the computed result and update the
@@ -959,6 +945,19 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         self->misses++;
         return result;
     }
+    if (PyErr_Occurred()) {
+        /* This is an unusual case since this same lookup
+           did not previously trigger an error during lookup.
+           Treat it the same as an error in user function
+           and return with the error set. */
+        Py_DECREF(key);
+        Py_DECREF(result);
+        return NULL;
+    }
+    /* This is the normal case.  The new key wasn't found before
+       user function call and it is still not there.  So we
+       proceed normally and update the cache with the new result. */
+
     assert(self->maxsize > 0);
     if (PyDict_GET_SIZE(self->cache) >= self->maxsize) {
         /* Since the cache is full, we need to evict an old key and add
@@ -995,71 +994,72 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
             Py_DECREF(popresult);
             Py_DECREF(link);
             Py_DECREF(key);
+            self->misses++;
+            return result;
         }
-        else if (popresult == NULL) {
+        if (popresult == NULL) {
             /* An error arose while trying to remove the oldest
                key (the one being evicted) from the cache.
                We restore the link to its original position
                as the oldest link.  Then we allow the error
                propagate upward; treating it the same as an
-               error arising in the user function.
-            */
+               error arising in the user function. */
             lru_cache_prepend_link(self, link);
             Py_DECREF(key);
             Py_DECREF(result);
             return NULL;
         }
-        else {
-            /* Keep a reference to the old key and old result to
-               prevent their ref counts from going to zero during the
-               update. That will prevent potentially arbitrary object
-               clean-up code (i.e. __del__) from running while we're
-               still adjusting the links. */
-            oldkey = link->key;
-            oldresult = link->result;
-
-            link->hash = hash;
-            link->key = key;
-            link->result = result;
-            if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
-                                          hash) < 0) {
-                /* Somehow the cache dict update failed.
-                   We no longer can restore the old link.
-                   Let the error propagate upward and
-                   leave the cache short one link. */
-                Py_DECREF(popresult);
-                Py_DECREF(link);
-                Py_DECREF(oldkey);
-                Py_DECREF(oldresult);
-                return NULL;
-            }
-            lru_cache_append_link(self, link);
-            Py_INCREF(result); /* for return */
-            Py_DECREF(popresult);
-            Py_DECREF(oldkey);
-            Py_DECREF(oldresult);
-        }
-    } else {
-        /* Put result in a new link at the front of the queue. */
-        link = (lru_list_elem *)PyObject_New(lru_list_elem,
-                                             &lru_list_elem_type);
-        if (link == NULL) {
-            Py_DECREF(key);
-            Py_DECREF(result);
-            return NULL;
-        }
+        /* Keep a reference to the old key and old result to
+           prevent their ref counts from going to zero during the
+           update. That will prevent potentially arbitrary object
+           clean-up code (i.e. __del__) from running while we're
+           still adjusting the links. */
+        oldkey = link->key;
+        oldresult = link->result;
 
         link->hash = hash;
         link->key = key;
         link->result = result;
         if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                       hash) < 0) {
+            /* Somehow the cache dict update failed.
+               We no longer can restore the old link.
+               Let the error propagate upward and
+               leave the cache short one link. */
+            Py_DECREF(popresult);
             Py_DECREF(link);
+            Py_DECREF(oldkey);
+            Py_DECREF(oldresult);
             return NULL;
         }
         lru_cache_append_link(self, link);
         Py_INCREF(result); /* for return */
+        Py_DECREF(popresult);
+        Py_DECREF(oldkey);
+        Py_DECREF(oldresult);
+        self->misses++;
+        return result;
+    }
+
+    /* Put result in a new link at the front of the queue. */
+    link = (lru_list_elem *)PyObject_New(lru_list_elem,
+                                         &lru_list_elem_type);
+    if (link == NULL) {
+        Py_DECREF(key);
+        Py_DECREF(result);
+        return NULL;
+    }
+
+    link->hash = hash;
+    link->key = key;
+    link->result = result;
+    if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
+                                  hash) < 0) {
+        Py_DECREF(link);
+        return NULL;
     }
+    lru_cache_append_link(self, link);
+    Py_INCREF(result); /* for return */
     self->misses++;
     return result;
 }

From e207d419f4a1df46dc798f59a3f76a887dff432e Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Thu, 24 Jan 2019 00:30:18 -0500
Subject: [PATCH 24/30] Arrange structure members in order of typical access

---
 Modules/_functoolsmodule.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 1b1bf513015221..90adeef56534d8 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -711,13 +711,14 @@ typedef PyObject *(*lru_cache_ternaryfunc)(struct lru_cache_object *, PyObject *
 
 typedef struct lru_cache_object {
     lru_list_elem root;  /* includes PyObject_HEAD */
-    Py_ssize_t maxsize;
-    PyObject *func;
     lru_cache_ternaryfunc wrapper;
+    int typed;
     PyObject *cache;
+    Py_ssize_t hits;
+    PyObject *func;
+    Py_ssize_t maxsize;
+    Py_ssize_t misses;
     PyObject *cache_info_type;
-    Py_ssize_t misses, hits;
-    int typed;
     PyObject *dict;
 } lru_cache_object;
 

From 614bfc84d8c6bafd9bea42a9a19092026b0359d0 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Thu, 24 Jan 2019 00:38:09 -0500
Subject: [PATCH 25/30] Order initial assignments and deallocations to match
 structure order

---
 Modules/_functoolsmodule.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 90adeef56534d8..82fb7cda06144f 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -1118,18 +1118,17 @@ lru_cache_new(PyTypeObject *type, PyObject *args, PyObject *kw)
         return NULL;
     }
 
-    obj->cache = cachedict;
     obj->root.prev = &obj->root;
     obj->root.next = &obj->root;
-    obj->maxsize = maxsize;
+    obj->wrapper = wrapper;
+    obj->typed = typed;
+    obj->cache = cachedict;
     Py_INCREF(func);
     obj->func = func;
-    obj->wrapper = wrapper;
     obj->misses = obj->hits = 0;
-    obj->typed = typed;
+    obj->maxsize = maxsize;
     Py_INCREF(cache_info_type);
     obj->cache_info_type = cache_info_type;
-
     return (PyObject *)obj;
 }
 
@@ -1163,10 +1162,10 @@ lru_cache_dealloc(lru_cache_object *obj)
     PyObject_GC_UnTrack(obj);
 
     list = lru_cache_unlink_list(obj);
-    Py_XDECREF(obj->func);
     Py_XDECREF(obj->cache);
-    Py_XDECREF(obj->dict);
+    Py_XDECREF(obj->func);
     Py_XDECREF(obj->cache_info_type);
+    Py_XDECREF(obj->dict);
     lru_cache_clear_list(list);
     Py_TYPE(obj)->tp_free(obj);
 }

From ed699751d39f8dee6ddc8ae353be669f1eca6274 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Thu, 24 Jan 2019 01:11:35 -0500
Subject: [PATCH 26/30] Update NEWS entry

---
 .../2019-01-19-17-01-43.bpo-35780.CLf7fT.rst      | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
index 9c652484526bfa..65fa48a9628551 100644
--- a/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
+++ b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
@@ -1,6 +1,9 @@
-Fix lru_cache() errors arising in recursive, reentrant, or multi-threaded
-code. The error could result in orphan links.
-
-Also apply space saving path for functions with a single positional
-argument.  In this common case, the space overhead of an lru cache entry is
-reduced by almost half.
+Fix lru_cache() errors arising in recursive, reentrant, or
+multi-threaded code. These errors could result in orphan links and in
+the cache being trapped in a state with fewer than the specified maximum
+number of links.  Fix handling of negative maxsize which should have
+been treated as zero.  Fix errors in toggling the "full" status flag.
+Fix misordering of links when errors are encountered.  Sync-up the C
+code and pure Python code for the space saving path in functions with a
+single positional argument. In this common case, the space overhead of
+an lru cache entry is reduced by almost half.

From f86e311863d2e3a87c094898c262c1320d9ac488 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Thu, 24 Jan 2019 02:35:08 -0500
Subject: [PATCH 27/30] Remove one more layer of if-logic to make the code
 easier to follow.

---
 Modules/_functoolsmodule.c | 127 ++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 67 deletions(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 82fb7cda06144f..7c2000f72fcee2 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -960,107 +960,100 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
        proceed normally and update the cache with the new result. */
 
     assert(self->maxsize > 0);
-    if (PyDict_GET_SIZE(self->cache) >= self->maxsize) {
-        /* Since the cache is full, we need to evict an old key and add
-           a new key.  Rather than free the old link and allocate a new
-           one, we reuse the link for the new key and result and move it
-           to front of the cache to mark it as recently used.
-
-           We try to assure all code paths (including errors) leave all
-           of the links in place.  Either the link is successfully
-           updated and moved or it is restored to its old position.
-           However if an unrecoverable error is found, it doesn't
-           make sense to reinsert the link, so we leave it out
-           and the cache will no longer register as full.
-        */
-        PyObject *oldkey, *oldresult, *popresult;
-
-        /* Extract the oldest item. */
-        assert(self->root.next != &self->root);
-        link = self->root.next;
-        lru_cache_extract_link(link);
-        /* Remove it from the cache.
-           The cache dict holds one reference to the link,
-           and the linked list holds yet one reference to it. */
-        popresult = _PyDict_Pop_KnownHash(self->cache,
-                                          link->key, link->hash,
-                                          Py_None);
-        if (popresult == Py_None) {
-            /* Getting here means that the user function call or
-               another thread has already removed the old key from the
-               dictionary.  This link is now an orpan.  Since we don't
-               want to leave the cache in an inconsistent state, we
-               don't restore the link.
-            */
-            Py_DECREF(popresult);
-            Py_DECREF(link);
-            Py_DECREF(key);
-            self->misses++;
-            return result;
-        }
-        if (popresult == NULL) {
-            /* An error arose while trying to remove the oldest
-               key (the one being evicted) from the cache.
-               We restore the link to its original position
-               as the oldest link.  Then we allow the error
-               propagate upward; treating it the same as an
-               error arising in the user function. */
-            lru_cache_prepend_link(self, link);
+    if (PyDict_GET_SIZE(self->cache) < self->maxsize) {
+        /* Cache is not full, so put the result in a new link */
+        link = (lru_list_elem *)PyObject_New(lru_list_elem,
+                                             &lru_list_elem_type);
+        if (link == NULL) {
             Py_DECREF(key);
             Py_DECREF(result);
             return NULL;
         }
-        /* Keep a reference to the old key and old result to
-           prevent their ref counts from going to zero during the
-           update. That will prevent potentially arbitrary object
-           clean-up code (i.e. __del__) from running while we're
-           still adjusting the links. */
-        oldkey = link->key;
-        oldresult = link->result;
 
         link->hash = hash;
         link->key = key;
         link->result = result;
         if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                       hash) < 0) {
-            /* Somehow the cache dict update failed.
-               We no longer can restore the old link.
-               Let the error propagate upward and
-               leave the cache short one link. */
-            Py_DECREF(popresult);
             Py_DECREF(link);
-            Py_DECREF(oldkey);
-            Py_DECREF(oldresult);
             return NULL;
         }
         lru_cache_append_link(self, link);
         Py_INCREF(result); /* for return */
+        self->misses++;
+        return result;
+    }
+    /* Since the cache is full, we need to evict an old key and add
+       a new key.  Rather than free the old link and allocate a new
+       one, we reuse the link for the new key and result and move it
+       to front of the cache to mark it as recently used.
+
+       We try to assure all code paths (including errors) leave all
+       of the links in place.  Either the link is successfully
+       updated and moved or it is restored to its old position.
+       However if an unrecoverable error is found, it doesn't
+       make sense to reinsert the link, so we leave it out
+       and the cache will no longer register as full.
+    */
+    PyObject *oldkey, *oldresult, *popresult;
+
+    /* Extract the oldest item. */
+    assert(self->root.next != &self->root);
+    link = self->root.next;
+    lru_cache_extract_link(link);
+    /* Remove it from the cache.
+       The cache dict holds one reference to the link,
+       and the linked list holds yet one reference to it. */
+    popresult = _PyDict_Pop_KnownHash(self->cache, link->key,
+                                      link->hash, Py_None);
+    if (popresult == Py_None) {
+        /* Getting here means that the user function call or another
+           thread has already removed the old key from the dictionary.
+           This link is now an orpan.  Since we don't want to leave the
+           cache in an inconsistent state, we don't restore the link. */
         Py_DECREF(popresult);
-        Py_DECREF(oldkey);
-        Py_DECREF(oldresult);
+        Py_DECREF(link);
+        Py_DECREF(key);
         self->misses++;
         return result;
     }
-
-    /* Put result in a new link at the front of the queue. */
-    link = (lru_list_elem *)PyObject_New(lru_list_elem,
-                                         &lru_list_elem_type);
-    if (link == NULL) {
+    if (popresult == NULL) {
+        /* An error arose while trying to remove the oldest key (the one
+           being evicted) from the cache.  We restore the link to its
+           original position as the oldest link.  Then we allow the
+           error propagate upward; treating it the same as an error
+           arising in the user function. */
+        lru_cache_prepend_link(self, link);
         Py_DECREF(key);
         Py_DECREF(result);
         return NULL;
     }
+    /* Keep a reference to the old key and old result to prevent their
+       ref counts from going to zero during the update. That will
+       prevent potentially arbitrary object clean-up code (i.e. __del__)
+       from running while we're still adjusting the links. */
+    oldkey = link->key;
+    oldresult = link->result;
 
     link->hash = hash;
     link->key = key;
     link->result = result;
     if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                   hash) < 0) {
+        /* Somehow the cache dict update failed.  We no longer can
+           restore the old link.  Let the error propagate upward and
+           leave the cache short one link. */
+        Py_DECREF(popresult);
         Py_DECREF(link);
+        Py_DECREF(oldkey);
+        Py_DECREF(oldresult);
         return NULL;
     }
     lru_cache_append_link(self, link);
     Py_INCREF(result); /* for return */
+    Py_DECREF(popresult);
+    Py_DECREF(oldkey);
+    Py_DECREF(oldresult);
     self->misses++;
     return result;
 }

From c6fa049c00ccec74207f4cbe2fda4854f26a053f Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Fri, 25 Jan 2019 08:11:09 -0500
Subject: [PATCH 28/30] Fix erroneous count of cache misses

---
 Lib/functools.py                                          | 2 +-
 .../next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst | 4 +++-
 Modules/_functoolsmodule.c                                | 8 ++------
 3 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/Lib/functools.py b/Lib/functools.py
index b455ecc429952a..6233c30c203ea7 100644
--- a/Lib/functools.py
+++ b/Lib/functools.py
@@ -581,6 +581,7 @@ def wrapper(*args, **kwds):
                     link[NEXT] = root
                     hits += 1
                     return result
+                misses += 1
             result = user_function(*args, **kwds)
             with lock:
                 if key in cache:
@@ -618,7 +619,6 @@ def wrapper(*args, **kwds):
                     # Use the cache_len bound method instead of the len() function
                     # which could potentially be wrapped in an lru_cache itself.
                     full = (cache_len() >= maxsize)
-                misses += 1
             return result
 
     def cache_info():
diff --git a/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
index 65fa48a9628551..d44488272170b3 100644
--- a/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
+++ b/Misc/NEWS.d/next/Library/2019-01-19-17-01-43.bpo-35780.CLf7fT.rst
@@ -6,4 +6,6 @@ been treated as zero.  Fix errors in toggling the "full" status flag.
 Fix misordering of links when errors are encountered.  Sync-up the C
 code and pure Python code for the space saving path in functions with a
 single positional argument. In this common case, the space overhead of
-an lru cache entry is reduced by almost half.
+an lru cache entry is reduced by almost half.  Fix counting of cache
+misses. In error cases, the miss count was out of sync with the actual
+number of times the underlying user function was called.
diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 7c2000f72fcee2..95ff47cef0d479 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -931,6 +931,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         Py_DECREF(key);
         return NULL;
     }
+    self->misses++;
     result = PyObject_Call(self->func, args, kwds);
     if (!result) {
         Py_DECREF(key);
@@ -940,10 +941,8 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
     if (testresult != NULL) {
         /* Getting here means that this same key was added to the cache
            during the PyObject_Call().  Since the link update is already
-           done, we need only return the computed result and update the
-           count of misses. */
+           done, we need only return the computed result. */
         Py_DECREF(key);
-        self->misses++;
         return result;
     }
     if (PyErr_Occurred()) {
@@ -980,7 +979,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         }
         lru_cache_append_link(self, link);
         Py_INCREF(result); /* for return */
-        self->misses++;
         return result;
     }
     /* Since the cache is full, we need to evict an old key and add
@@ -1014,7 +1012,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         Py_DECREF(popresult);
         Py_DECREF(link);
         Py_DECREF(key);
-        self->misses++;
         return result;
     }
     if (popresult == NULL) {
@@ -1054,7 +1051,6 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
     Py_DECREF(popresult);
     Py_DECREF(oldkey);
     Py_DECREF(oldresult);
-    self->misses++;
     return result;
 }
 

From 14281bb165546bbdbf5962a02d1bd14becc8e8ca Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Fri, 25 Jan 2019 08:33:10 -0500
Subject: [PATCH 29/30] Add more comments on reentrancy issues

---
 Modules/_functoolsmodule.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 95ff47cef0d479..77424fa67f662c 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -972,6 +972,11 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
         link->hash = hash;
         link->key = key;
         link->result = result;
+        /* What is really needed here is a SetItem variant with a "no clobber"
+           option.  If the __eq__ call triggers a reentrant call that adds
+           this same key, then this setitem call will update the cache dict
+           with this new link, leaving the old link as an orphan (i.e. not
+           having a cache dict entry that refers to it). */
         if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                       hash) < 0) {
             Py_DECREF(link);
@@ -1035,6 +1040,11 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
     link->hash = hash;
     link->key = key;
     link->result = result;
+    /* Note:  The link is being added to the cache dict without the
+       prev and next fields set to valid values.   We have to wait
+       for successful insertion in the cache dict before adding the
+       link to the linked list.  Otherwise, the potentially reentrant
+       __eq__ call could cause the then ophan link to be visited. */
     if (_PyDict_SetItem_KnownHash(self->cache, key, (PyObject *)link,
                                   hash) < 0) {
         /* Somehow the cache dict update failed.  We no longer can

From 3b04baa085a72cf89f73dbd2f7127d338b7308b9 Mon Sep 17 00:00:00 2001
From: Raymond Hettinger <raymond.hettinger@gmail.com>
Date: Fri, 25 Jan 2019 08:42:11 -0500
Subject: [PATCH 30/30] Guard against exotic cases until they can be proven not
 to occur

---
 Modules/_functoolsmodule.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c
index 77424fa67f662c..141210204ca577 100644
--- a/Modules/_functoolsmodule.c
+++ b/Modules/_functoolsmodule.c
@@ -959,7 +959,9 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
        proceed normally and update the cache with the new result. */
 
     assert(self->maxsize > 0);
-    if (PyDict_GET_SIZE(self->cache) < self->maxsize) {
+    if (PyDict_GET_SIZE(self->cache) < self->maxsize ||
+        self->root.next == &self->root)
+    {
         /* Cache is not full, so put the result in a new link */
         link = (lru_list_elem *)PyObject_New(lru_list_elem,
                                              &lru_list_elem_type);