-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
MemoryError in test_strptime #69217
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
Comments
All builds on edelsohn-fedora-ppc64 are red starting from build 55 for 3.x and build 41 for 3.5. 3.4 is green. ====================================================================== Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/test/test_strptime.py", line 565, in test_TimeRE_recreation
_strptime._strptime_time('10', '%d')
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/_strptime.py", line 494, in _strptime_time
tt = _strptime(data_string, format)[0]
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/_strptime.py", line 312, in _strptime
_TimeRE_cache = TimeRE()
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/_strptime.py", line 190, in __init__
self.locale_time = LocaleTime()
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/_strptime.py", line 75, in __init__
self.__calc_am_pm()
File "/home/shager/cpython-buildarea/3.5.edelsohn-fedora-ppc64/build/Lib/_strptime.py", line 114, in __calc_am_pm
am_pm.append(time.strftime("%p", time_tuple).lower())
MemoryError |
Any clue here? Is this unaligned access? |
PPC64 is not a strict alignment system. The system is running a non-recent release of Fedora, so it could be a bad interaction with libc. |
I ran test_strptime & test_os with tracemalloc: test_os memory peak is higher than test_strptime memory peak (I ran the two tests independently). So the bug cannot be reproduce on my Fedora 22 (x86_64). |
Oh wait, the VmPeak is *much* high when running test_strptime:
1 GB should be enough for everybody: $ bash -c 'ulimit -v 1000000; ./python -m test -v test_strptime'
.. ====================================================================== Traceback (most recent call last):
...
File "/home/haypo/prog/python/default/Lib/_strptime.py", line 75, in __init__
self.__calc_am_pm()
File "/home/haypo/prog/python/default/Lib/_strptime.py", line 114, in __calc_am_pm
am_pm.append(time.strftime("%p", time_tuple).lower())
MemoryError |
It's a regression introduced by c31dad22c80d (Issue bpo-24917). |
I would blame this change: <https://hg.python.org/cpython/rev/c31dad22c80d/#l3.41\>. The rest should not have any effect on Linux. |
Script to reproduce the issue: import time
import locale
import pprint
time_tuple = time.struct_time((1999,3,17,1,44,55,2,76,0))
p1 = time.strftime("%p", time_tuple)
print("current LC_TIME", repr(p1))
locale.setlocale(locale.LC_TIME, ('de_DE', 'UTF8'))
p2 = time.strftime("%p", time_tuple)
print("de_DE (UTF8)", repr(p2)) Output: $ python3.4 bug.py
current LC_TIME 'AM'
de_DE (UTF8) '' The problem is that strftime()/wcsftime() *can* return 0, it's not an error. Whereas c31dad22c80d considers that if buflen is 0 but fmtlen is smaller than 5, we must retry with a larger buffer. |
Again, this issue remembers me the idea of rewriting strftime() in Python. We already have _strptime.py. |
Sorry Larry. I'll fix it. |
Steve, it seems to me that for MSVC the EINVAL test should come first:
_Py_BEGIN_SUPPRESS_IPH
olderr = errno;
errno = 0;
buflen = format_time(outbuf, i, fmt, &buf);
err = errno;
errno = olderr;
_Py_END_SUPPRESS_IPH
if (buflen == 0 && err == EINVAL) {
PyErr_SetString(PyExc_ValueError, "Invalid format string");
break;
} Then the old test could be restored, i.e. (buflen > 0 || i >= 256 * fmtlen). |
@eryksun - that's exactly what I've just done :) Unfortunately, I don't have a _locale module, so I can't do Victor's test. Attached my patch in case Victor is around to test it. |
My test runs the following command on Linux (Fedora 22): make && bash -c 'ulimit -v 1000000; ./python -u -m test -v test_strptime'
|
Thanks. I'll submit a PR for 3.5.0 later tonight - can't seem to clone Larry's repo successfully at work for some reason. |
Does this need a Misc/NEWS entry? |
Probably should, since the fix that caused it was in for rc3. There's no section for 3.5.0 final yet though (that's my excuse, anyway :) ) |
New changeset bd7aed300a1b by Steve Dower in branch '3.5': New changeset 706b9eaba734 by Steve Dower in branch '3.5': New changeset cceaccb6ec5a by Steve Dower in branch '3.5': |
The test doesn't fail anymore on buildbots. I ran manually test_strptime with a memory limit of 1 GB on Python 3.5 and 3.6: the test pass. Can we close the issue? |
Done. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: