classification
Title: time.tzset does not reset _strptime's locale time cache
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: belopolsky, berker.peksag, mibanescu, python-dev, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2009-07-13 17:33 by mibanescu, last changed 2015-12-12 08:25 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
ttime.py mibanescu, 2009-07-13 17:33 Reproducer for the problem.
_strptime.py.patch mibanescu, 2010-07-12 13:55
issue6478.patch berker.peksag, 2012-10-26 18:56 review
issue6478_v2.patch berker.peksag, 2012-10-27 16:45 review
issue6478_v3.diff berker.peksag, 2013-02-09 01:45 review
strptime_cache_timezone.patch serhiy.storchaka, 2015-11-23 10:42 review
Messages (15)
msg90497 - (view) Author: Mihai Ibanescu (mibanescu) Date: 2009-07-13 17:33
If one changes from one timezone to another within the same python
process, and if one tries to parse a time string that includes the
timezone, the library malfunctions.

See attached script for a reproducer.

The problem is that, even though time.tzset() is called, the LocaleTime
persisted in the TimeRE global is not reset. In my example, the EDT
timezone name, compiled from the previous TZ variable, is not valid
anymore in the 'Pacific/Fiji' timezone.

To witness the behavior, run the attached script with no arguments. It
will parse the time in the America/New_York timezone just fine.

Then run it with an argument (like python ttime.py 1). It will first
prime the _strptime cache in the Pacific/Fiji timezone, then attempt to
parse the same time string in the America/New_York timezone.

Finally, you can change the "if 0" to "if 1" for working around the problem.

This has been verified in 2.4.4 and 2.6.1 (did not have a chance to
verify it against python 2.6.2 yet).
msg110086 - (view) Author: Mihai Ibanescu (mibanescu) Date: 2010-07-12 13:55
Proposed patch attached.
msg110091 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-07-12 15:05
Thank you for the bug report and a patch.

This does look like a bug and proposed patch seems to be the simplest way to fix it.  It is unfortunate that the entire TimeRE cache needs to be recalculated when only 'Z' entry is invalidated by TZ change.  Please consider separating lang and TZ change checks and not reinitialize TimeRE cache object if only TZ changes, just replace the 'Z' entry. 

 The patch also needs unit tests.  It should be straightforward to convert ttime.py into a test case.  Please add a test case for both test_time and test_datetime.  Note that in 3.2 datetime.strptime() will process '%Z' when '%z' is also present.  For example,


>>> datetime.strptime('Fri Jul 25 13:26:29 EDT -0500 2008', '%a %b %d %H:%M:%S %Z %z %Y')
datetime.datetime(2008, 7, 25, 13, 26, 29, tzinfo=datetime.timezone(datetime.timedelta(-1, 68400), 'EDT'))


Please make sure to restore environment variables in test cleanup.

Also a nit on the implementation: cls is unused in _get_timezone(cls), so it would be more appropriate to make it staticmethod instead of classmethod.  And tzset should probably be called inside _get_timezone.
msg173879 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2012-10-26 18:56
Here is the second patch with Alexander Belopolsky's comments addressed.
msg173880 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-10-26 19:04
The patch looks good, but I have a few comments on the test:

1. Does it work on Windows?  It seems to rely on Olson's TZ names.
2. Please use @run_with_tz decorator.
msg173905 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2012-10-26 23:33
Thanks for the review.

> 1. Does it work on Windows?  It seems to rely on Olson's TZ names.

Is `time.tzset()` function only available on Unix?

See: http://docs.python.org/dev/library/time.html#time.tzset

> 2. Please use @run_with_tz decorator.

I will update my patch to use @run_with_tz decorator.
msg173941 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2012-10-27 16:45
> 2. Please use @run_with_tz decorator.

Done. Patch attached.
msg174824 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2012-11-04 18:07
Alexander: Did you have a chance to review the test?
msg174826 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2012-11-04 19:26
issue6478_v2.patch looks good to me.  There is a long line in _strptime.py which I will fix before committing.
msg181711 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2013-02-09 01:45
> There is a long line in _strptime.py which I will fix before
> committing.

Ops, fixed.

Also, I've found a better place for the test in Lib/test/test_strptime.py
and moved it to the test_strptime.CacheTests.

When I applied the patch, test_bad_timezone test failed because of
this code in Lib/_strptime.py:

    @staticmethod
    def _get_timezone():
        try:
            time.tzset()
        except AttributeError:
            pass
        # ...

time.tzset() always resets time.tzname[1], so this part of the code
in test_bad_timezone[2] didn't work.

    tzname = time.tzname[0]
    time.tzname = (tzname, tzname)

I've fixed test_bad_timezone test by mocking the new
_strptime._TimeRE_cache.locale_time._get_timezone method.

[1] http://hg.python.org/cpython/file/default/Modules/timemodule.c#l827
[2] http://hg.python.org/cpython/file/default/Lib/test/test_strptime.py#l320
msg209738 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-30 20:55
May be System Z Linux buildbot fails due to this bug (http://buildbot.python.org/all/builders/System%20Z%20Linux%203.x/builds/1154/steps/test/logs/stdio):

======================================================================
ERROR: test_strptime (test.datetimetester.TestDateTime_Pure)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-zlinux-z/build/Lib/test/datetimetester.py", line 1879, in test_strptime
    dt = strptime(dtstr, "%z %Z")
  File "/home/dje/cpython-buildarea/3.x.edelsohn-zlinux-z/build/Lib/datetime.py", line 1593, in strptime
    return _strptime._strptime_datetime(cls, date_string, format)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-zlinux-z/build/Lib/_strptime.py", line 500, in _strptime_datetime
    tt, fraction = _strptime(data_string, format)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-zlinux-z/build/Lib/_strptime.py", line 337, in _strptime
    (data_string, format))
ValueError: time data '-0500 EST' does not match format '%z %Z'

----------------------------------------------------------------------
msg229171 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-12 16:07
Berker: the patch doesn't apply cleanly any more.  Also, about the test_bad_timezone modification...what about the previous check that the tzname wasn't in UTC/GMT?  Isn't that still needed?  Or perhaps better yet, an additional @run_with_tz decorator?  (Note that I don't really *understand* this code, so I may be totally off base here... :)
msg255146 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-11-23 10:42
I forgot about this issue and proposed similar patch for issue25168. It doesn't recalculate timezone values (a tuple of frozensets) on every _strptime call, but uses cheaper tests for time.tzname and time.daylight.

issue6478_v3.diff has a bug, it doesn't clear _regex_cache after changing _TimeRE_cache. And I doubt that after fixing this the benefit of not recreating the entire _TimeRE_cache will be worth the complication of the code.

Looks as this bug is a cause of random order depending tests failure (issue22067).
msg255835 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-12-03 20:28
New changeset 2ae5c51c5dea by Serhiy Storchaka in branch '2.7':
Issue #6478: _strptime's regexp cache now is reset after changing timezone
https://hg.python.org/cpython/rev/2ae5c51c5dea

New changeset 4b0a4da1aa27 by Serhiy Storchaka in branch '3.4':
Issue #6478: _strptime's regexp cache now is reset after changing timezone
https://hg.python.org/cpython/rev/4b0a4da1aa27

New changeset 5fa855d20624 by Serhiy Storchaka in branch '3.5':
Issue #6478: _strptime's regexp cache now is reset after changing timezone
https://hg.python.org/cpython/rev/5fa855d20624

New changeset ea576db13827 by Serhiy Storchaka in branch 'default':
Issue #6478: _strptime's regexp cache now is reset after changing timezone
https://hg.python.org/cpython/rev/ea576db13827
msg256244 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2015-12-11 23:29
Thanks Serhiy. Can we close this now?
History
Date User Action Args
2015-12-12 08:25:22serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2015-12-11 23:29:14berker.peksagsetmessages: + msg256244
2015-12-03 20:28:15python-devsetnosy: + python-dev
messages: + msg255835
2015-11-23 10:42:15serhiy.storchakasetfiles: + strptime_cache_timezone.patch

stage: needs patch -> patch review
messages: + msg255146
versions: + Python 3.5, Python 3.6, - Python 3.3
2014-10-12 16:07:19r.david.murraysetnosy: + r.david.murray

messages: + msg229171
stage: commit review -> needs patch
2014-01-30 20:55:41serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg209738
2013-02-09 01:45:15berker.peksagsetfiles: + issue6478_v3.diff

messages: + msg181711
2012-11-04 19:26:30belopolskysetstage: test needed -> commit review
messages: + msg174826
versions: - Python 3.2
2012-11-04 18:07:17berker.peksagsetmessages: + msg174824
2012-10-27 16:45:05berker.peksagsetfiles: + issue6478_v2.patch

messages: + msg173941
2012-10-26 23:33:39berker.peksagsetmessages: + msg173905
2012-10-26 19:04:08belopolskysetmessages: + msg173880
2012-10-26 18:56:54berker.peksagsetfiles: + issue6478.patch
versions: + Python 3.3, Python 3.4, - Python 3.1
nosy: + berker.peksag

messages: + msg173879
2010-07-12 15:05:47belopolskysetmessages: + msg110091
stage: needs patch -> test needed
2010-07-12 13:58:20belopolskysetassignee: belopolsky
2010-07-12 13:55:15mibanescusetfiles: + _strptime.py.patch
keywords: + patch
messages: + msg110086
2010-07-10 23:53:56BreamoreBoysetnosy: + belopolsky
stage: needs patch

versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6, Python 2.4
2009-07-13 17:33:43mibanescucreate