Issue28969
Created on 2016-12-14 10:49 by Nicolas Savoire, last changed 2022-04-11 14:58 by admin. 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) * | 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) * | 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) * | 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) * | 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) * | Date: 2016-12-28 09:15 | |
What solution do you prefer Raymond? |
|||
| msg284169 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | 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) * | 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 (methane) * | Date: 2017-01-09 18:39 | |
LGTM |
|||
| msg285338 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | 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 |
| 2022-04-11 14:58:40 | admin | set | github: 73155 |
| 2017-04-24 12:01:08 | serhiy.storchaka | set | pull_requests: - pull_request855 |
| 2017-04-24 10:23:35 | jcea | set | nosy:
+ jcea |
| 2017-03-31 16:36:10 | dstufft | set | pull_requests: + pull_request855 |
| 2017-01-12 19:44:02 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: commit review -> resolved |
| 2017-01-12 18:03:41 | serhiy.storchaka | set | messages: + msg285338 |
| 2017-01-12 15:37:23 | serhiy.storchaka | set | assignee: rhettinger -> serhiy.storchaka stage: patch review -> commit review |
| 2017-01-09 18:39:29 | methane | set | nosy:
+ methane messages: + msg285062 |
| 2017-01-09 07:31:55 | serhiy.storchaka | set | messages: + msg285024 |
| 2017-01-03 14:23:51 | vstinner | set | nosy:
+ vstinner |
| 2016-12-28 10:36:02 | serhiy.storchaka | set | files:
+ lru_cache-dict-pop-3.5-2.patch messages: + msg284169 |
| 2016-12-28 09:15:30 | serhiy.storchaka | set | assignee: serhiy.storchaka -> rhettinger messages: + msg284168 |
| 2016-12-16 23:11:42 | serhiy.storchaka | set | files:
+ lru_cache-dict-pop-simpler-3.5.patch messages: + msg283448 |
| 2016-12-16 21:30:39 | ned.deily | set | messages: + msg283440 |
| 2016-12-16 21:16:08 | rhettinger | set | nosy:
+ ned.deily messages: + msg283439 |
| 2016-12-16 00:54:44 | josh.r | set | nosy:
+ josh.r |
| 2016-12-15 12:52:39 | serhiy.storchaka | set | files: + lru_cache-dict-pop-3.5.patch |
| 2016-12-15 12:51:56 | serhiy.storchaka | set | files: - lru_cache-dict-pop-3.5.patch |
| 2016-12-15 12:16:52 | serhiy.storchaka | set | type: crash -> behavior |
| 2016-12-15 12:07:36 | serhiy.storchaka | set | files:
+ lru_cache-dict-pop-3.5.patch keywords: + patch messages: + msg283292 stage: needs patch -> patch review |
| 2016-12-15 11:12:33 | serhiy.storchaka | set | title: fnmatch is not threadsafe -> lru_cache is not threadsafe |
| 2016-12-15 10:55:29 | serhiy.storchaka | set | keywords:
+ 3.5regression assignee: serhiy.storchaka stage: needs patch versions: + Python 3.6, Python 3.7 |
| 2016-12-15 10:36:25 | peter.otten | set | nosy:
+ rhettinger, peter.otten, serhiy.storchaka messages: + msg283279 |
| 2016-12-14 10:49:24 | Nicolas Savoire | create | |