lru_cache is not threadsafe

[NicolasSavoire]

BPO	28969
Nosy	@rhettinger, @jcea, @vstinner, @ned-deily, @methane, @serhiy-storchaka, @MojoVampire
Files	test.py lru_cache-dict-pop-3.5.patch: Patch for 3.5 lru_cache-dict-pop-simpler-3.5.patch lru_cache-dict-pop-3.5-2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-01-12.19:44:02.995>
created_at = <Date 2016-12-14.10:49:24.502>
labels = ['3.7', 'type-bug', 'library']
title = 'lru_cache is not threadsafe'
updated_at = <Date 2017-04-24.12:01:08.839>
user = 'https://bugs.python.org/NicolasSavoire'

bugs.python.org fields:

activity = <Date 2017-04-24.12:01:08.839>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-01-12.19:44:02.995>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-12-14.10:49:24.502>
creator = 'Nicolas Savoire'
dependencies = []
files = ['45895', '45914', '45931', '46062']
hgrepos = []
issue_num = 28969
keywords = ['patch', '3.5regression']
message_count = 11.0
messages = ['283185', '283279', '283292', '283439', '283440', '283448', '284168', '284169', '285024', '285062', '285338']
nosy_count = 9.0
nosy_names = ['rhettinger', 'jcea', 'peter.otten', 'vstinner', 'ned.deily', 'methane', 'serhiy.storchaka', 'josh.r', 'Nicolas Savoire']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue28969'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[NicolasSavoire]

With python3.5, fnmatch appears not to be thrad safe. It fails with the following exception:

Traceback (most recent call last):
  File "test.py", line 18, in <module>
    f.result()
  File "/opt/anaconda3/lib/python3.5/concurrent/futures/_base.py", line 405, in result
    return self.__get_result()
  File "/opt/anaconda3/lib/python3.5/concurrent/futures/_base.py", line 357, in __get_result
    raise self._exception
  File "/opt/anaconda3/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "test.py", line 12, in foo
    fnmatch(ref, s)
  File "/opt/anaconda3/lib/python3.5/fnmatch.py", line 36, in fnmatch
    return fnmatchcase(name, pat)
  File "/opt/anaconda3/lib/python3.5/fnmatch.py", line 70, in fnmatchcase
    match = _compile_pattern(pat)
KeyError: ('SXJ8WZ9F7Z', <class 'str'>)

Here is a minimal example reproducing the issue:

import concurrent.futures
from fnmatch import fnmatch
import random
import string

def str_generator(size=10, chars=string.ascii_uppercase + string.digits):
   return ''.join(random.choice(chars) for _ in range(size))


def foo(strs, ref):
    for s in strs:
        fnmatch(ref, s)

some_strings = [str_generator() for _ in range(500)]
with concurrent.futures.ThreadPoolExecutor() as executor:
    futures = [executor.submit(foo, some_strings, some_strings[0]) for _ in range(8)]
    for f in futures:
        f.result()

$ python3 --version
Python 3.5.2 :: Anaconda 4.2.0 (64-bit)

[peterotten]

Here's another way to reproduce the error. The problem seems to be in the C implementation of _lru_cache_wrapper() / bounded_lru_cache_wrapper().

$ cat test.py
import functools
import threading
import time

@functools.lru_cache(maxsize=2)
def f(v):
    time.sleep(.01)

threads = [threading.Thread(target=f, args=(v,)) for v in [1,2,2,3,2]]
for t in threads:
    t.start()

$ ./python test.py
Exception in thread Thread-5:
Traceback (most recent call last):
  File "/somewhere/Lib/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/somewhere/Lib/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
KeyError: (2,)

$ ./python
Python 3.7.0a0 (default:654ec6ed3225+, Dec 15 2016, 11:24:30) 
[GCC 4.8.4] on linux

[serhiy-storchaka]

Thank you for your example Peter.

Proposed patch fixes the KeyError. It introduces new private C API _PyDict_Pop_KnownHash(). It is like _PyDict_Pop(), but with precomputed hash.

[rhettinger]

Ned, can this be considered for inclusion in Python 3.6? ISTM that this problematic in that it may already be causing hard-to-diagnose bugs in existing applications.

[ned-deily]

Before it could be considered for 3.6.0, it needs to be reviewed, pushed to the 3.6 branch for 3.6.1, and have some buildbot exposure. 3.6.0rc2 is in the process of manufacturing already. Since this is a fairly extensive change, I think it would not be appropriate to throw it into a final release without prior release exposure. Since it's also a bug in 3.5 and not a 3.6 release regression, I think it should wait for 3.6.1 unless we have a compelling reason to do a 3.6.0rc3.

[serhiy-storchaka]

An alternative patch changes semantic of _PyDict_Pop() with deflt == NULL, but makes the code simpler.

[serhiy-storchaka]

What solution do you prefer Raymond?