Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lru_cache is not threadsafe #73155

Closed
NicolasSavoire mannequin opened this issue Dec 14, 2016 · 11 comments
Closed

lru_cache is not threadsafe #73155

NicolasSavoire mannequin opened this issue Dec 14, 2016 · 11 comments
Assignees
@serhiy-storchaka
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@NicolasSavoire
Copy link
Mannequin

NicolasSavoire mannequin commented Dec 14, 2016

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
    Copy link
    Mannequin Author

    NicolasSavoire mannequin commented Dec 14, 2016

    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)

    @NicolasSavoire NicolasSavoire mannequin added type-crash A hard crash of the interpreter, possibly with a core dump stdlib Python modules in the Lib dir labels Dec 14, 2016
    @peterotten
    Copy link
    Mannequin

    peterotten mannequin commented Dec 15, 2016

    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 serhiy-storchaka added the 3.7 (EOL) end of life label Dec 15, 2016
    @serhiy-storchaka serhiy-storchaka self-assigned this Dec 15, 2016
    @serhiy-storchaka serhiy-storchaka changed the title fnmatch is not threadsafe lru_cache is not threadsafe Dec 15, 2016
    @serhiy-storchaka
    Copy link
    Member

    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.

    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Dec 15, 2016
    @rhettinger
    Copy link
    Contributor

    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
    Copy link
    Member

    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
    Copy link
    Member

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

    @serhiy-storchaka
    Copy link
    Member

    What solution do you prefer Raymond?

    @serhiy-storchaka
    Copy link
    Member

    Antoine noticed that the first patch doesn't need special sentinel object. Thus it can be simplified.

    @serhiy-storchaka
    Copy link
    Member

    With Antoine's suggestion lru_cache-dict-pop-simpler-3.5.patch no longer has an advantage. Just ignore it.

    @methane
    Copy link
    Member

    methane commented Jan 9, 2017

    LGTM

    @serhiy-storchaka
    Copy link
    Member

    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
...

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @methane @rhettinger @serhiy-storchaka @ned-deily