classification
Title: lru_cache is not threadsafe
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Nicolas Savoire, inada.naoki, jcea, josh.r, ned.deily, peter.otten, rhettinger, serhiy.storchaka, vstinner
Priority: normal Keywords: 3.5regression, patch

Created on 2016-12-14 10:49 by Nicolas Savoire, last changed 2017-04-24 12:01 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
test.py Nicolas Savoire, 2016-12-14 10:49
lru_cache-dict-pop-3.5.patch serhiy.storchaka, 2016-12-15 12:52 Patch for 3.5 review
lru_cache-dict-pop-simpler-3.5.patch serhiy.storchaka, 2016-12-16 23:11 review
lru_cache-dict-pop-3.5-2.patch serhiy.storchaka, 2016-12-28 10:36 review
Messages (11)
msg283185 - (view) Author: Nicolas Savoire (Nicolas Savoire) Date: 2016-12-14 10:49
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)
msg283279 - (view) Author: Peter Otten (peter.otten) * Date: 2016-12-15 10:36
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
msg283292 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-15 12:07
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.
msg283439 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-12-16 21:16
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.
msg283440 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-12-16 21:30
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.
msg283448 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-16 23:11
An alternative patch changes semantic of _PyDict_Pop() with deflt == NULL, but makes the code simpler.
msg284168 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-28 09:15
What solution do you prefer Raymond?
msg284169 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-12-28 10:36
Antoine noticed that the first patch doesn't need special sentinel object. Thus it can be simplified.
msg285024 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-09 07:31
With Antoine's suggestion lru_cache-dict-pop-simpler-3.5.patch no longer has an advantage. Just ignore it.
msg285062 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2017-01-09 18:39
LGTM
msg285338 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-12 18:03
Thanks Inada.

Committed in 9314aebc9674, 5724c3ddf25b and 9c852878a998. There were errors during pushing:

remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 9 changesets with 15 changes to 5 files
remote: buildbot: change(s) sent successfully
remote: Traceback (most recent call last):
remote:   File "/srv/hg/repos/hooks/hgroundup.py", line 56, in update_issue
remote:     _update_issue(*args, **kwargs)
remote:   File "/srv/hg/repos/hooks/hgroundup.py", line 108, in _update_issue
remote:     s.login(username, password)
remote:   File "/usr/lib/python2.7/smtplib.py", line 610, in login
remote:     AUTH_PLAIN + " " + encode_plain(user, password))
remote:   File "/usr/lib/python2.7/smtplib.py", line 394, in docmd
remote:     return self.getreply()
remote:   File "/usr/lib/python2.7/smtplib.py", line 365, in getreply
remote:     + str(e))
remote: SMTPServerDisconnected: Connection unexpectedly closed: timed out
remote: error: changegroup.roundup hook raised an exception: Connection unexpectedly closed: timed out
remote: notified python-checkins@python.org of incoming changeset 9314aebc9674

remote: Traceback (most recent call last):
remote:   File "/srv/hg/repos/hooks/mail.py", line 166, in incoming
remote:     return _incoming(ui, repo, **kwargs)
remote:   File "/srv/hg/repos/hooks/mail.py", line 157, in _incoming
remote:     send(smtp, subj, sender, to, '\n'.join(body) + '\n')
remote:   File "/srv/hg/repos/hooks/mail.py", line 37, in send
remote:     smtp.sendmail(sender, to, msg.as_string())
remote:   File "/usr/lib/python2.7/smtplib.py", line 748, in sendmail
remote:     (code, resp) = self.data(msg)
remote:   File "/usr/lib/python2.7/smtplib.py", line 511, in data
remote:     (code, msg) = self.getreply()
remote:   File "/usr/lib/python2.7/smtplib.py", line 365, in getreply
remote:     + str(e))
remote: SMTPServerDisconnected: Connection unexpectedly closed: timed out
remote: error: incoming.notify hook raised an exception: Connection unexpectedly closed: timed out
...
History
Date User Action Args
2017-04-24 12:01:08serhiy.storchakasetpull_requests: - pull_request855
2017-04-24 10:23:35jceasetnosy: + jcea
2017-03-31 16:36:10dstufftsetpull_requests: + pull_request855
2017-01-12 19:44:02serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2017-01-12 18:03:41serhiy.storchakasetmessages: + msg285338
2017-01-12 15:37:23serhiy.storchakasetassignee: rhettinger -> serhiy.storchaka
stage: patch review -> commit review
2017-01-09 18:39:29inada.naokisetnosy: + inada.naoki
messages: + msg285062
2017-01-09 07:31:55serhiy.storchakasetmessages: + msg285024
2017-01-03 14:23:51vstinnersetnosy: + vstinner
2016-12-28 10:36:02serhiy.storchakasetfiles: + lru_cache-dict-pop-3.5-2.patch

messages: + msg284169
2016-12-28 09:15:30serhiy.storchakasetassignee: serhiy.storchaka -> rhettinger
messages: + msg284168
2016-12-16 23:11:42serhiy.storchakasetfiles: + lru_cache-dict-pop-simpler-3.5.patch

messages: + msg283448
2016-12-16 21:30:39ned.deilysetmessages: + msg283440
2016-12-16 21:16:08rhettingersetnosy: + ned.deily
messages: + msg283439
2016-12-16 00:54:44josh.rsetnosy: + josh.r
2016-12-15 12:52:39serhiy.storchakasetfiles: + lru_cache-dict-pop-3.5.patch
2016-12-15 12:51:56serhiy.storchakasetfiles: - lru_cache-dict-pop-3.5.patch
2016-12-15 12:16:52serhiy.storchakasettype: crash -> behavior
2016-12-15 12:07:36serhiy.storchakasetfiles: + lru_cache-dict-pop-3.5.patch
keywords: + patch
messages: + msg283292

stage: needs patch -> patch review
2016-12-15 11:12:33serhiy.storchakasettitle: fnmatch is not threadsafe -> lru_cache is not threadsafe
2016-12-15 10:55:29serhiy.storchakasetkeywords: + 3.5regression
assignee: serhiy.storchaka
stage: needs patch
versions: + Python 3.6, Python 3.7
2016-12-15 10:36:25peter.ottensetnosy: + rhettinger, peter.otten, serhiy.storchaka
messages: + msg283279
2016-12-14 10:49:24Nicolas Savoirecreate