classification
Title: timemodule.c: Python loses current timezone
Type: behavior Stage: patch review
Components: Extension Modules Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: out of date
Dependencies: Superseder: Time zone-capable variant of time.localtime
View: 1667546
Assigned To: loewis Nosy List: Stephen.White, belopolsky, brian.curtin, grantbow, jonsiddle, loewis, pboddie, quale, r.david.murray, rhettinger
Priority: normal Keywords: needs review, patch

Created on 2003-06-30 02:18 by quale, last changed 2015-10-02 21:30 by belopolsky. This issue is now closed.

Files
File name Uploaded Description Edit
timemodule.c.patch quale, 2003-06-30 02:18 timezone handling patch to timemodule.c
gettmargs.patch quale, 2003-08-09 17:16 corrected patch
gettmargs-test.py quale, 2003-08-21 16:57 unit tests for gettmargs patch
issue762963.diff brian.curtin, 2010-01-18 03:02 patch against r77593 (trunk) review
Messages (18)
msg44158 - (view) Author: Doug Quale (quale) Date: 2003-06-30 02:18
Python routines in timemodule.c sometimes force glibc
to use GMT instead of the correct timezone.

I think this is a longstanding bug, but I have only
looked at Python 2.2 and 2.33b1. This bug has been
reported before (510218) but it was misdiagnosed and
closed. It also came up recently in on the python-list
(http://aspn.activestate.com/ASPN/Mail/Message/python-list/1564384)
where it was also misdiagnosed.

Standard C and Python use a struct tm with 9 values. 
BSD influenced C libraries like glibc have an extra
field (tm_gmtoff) that keeps the offset from UTC. 
BSD-style struct tm values know the correct timezone
associated with the time, but Python and Standard C
struct tm values don't so they can only assume the
current timezone.

Ideally Python would always treat stuct tm-style time
tuples as being associated with the current timezone.
Unfortunately in many cases Python forces glibc to use
GMT rather than the current timezone.  You can see the
problem most graphically with the %z format in
strftime().  In the transcript Python incorrectly gives
the timezone offset as +0000 even though it gets the
timezone name (CDT) correct:

$ python2.3
Python 2.3b1+ (#2, Jun  4 2003, 03:03:32) 
[GCC 3.3 (Debian)] on linux2
Type "help", "copyright", "credits" or "license" for
more information.
>>> import time
>>> now = time.localtime()
>>> print now
(2003, 6, 29, 20, 3, 52, 6, 180, 1)
>>> RFC822 = '%a, %d %b %Y %T %z'
>>> print time.strftime(RFC822, now)
Sun, 29 Jun 2003 20:03:52 +0000
>>> print time.strftime(RFC822)
Sun, 29 Jun 2003 20:05:27 -0500
>>> print time.strftime('%Z', now)
CDT

The correct timezone is CDT and the correct offset is
-0500. Notice that when time.strftime() computes the
current time it it gets the correct timezone offset,
but when the struct tm tuple is passed in it thinks the
timezone offset is 0.  (glibc does know that the
correct timezone name is CDT even when passed a bad
struct tm value, but that's not surprising since the
timezone name is not stored in struct tm and it is not
computed from timezone offset.)

The problem is in the gettmargs() function in
timemodule.c. When getmargs() parses a Python tm tuple
it leaves the tm_gmtoff field zeroed. This specifically
tells glibc that the timezone offset is 0, which is
wrong for most people. (BSD libc may also be affected.)
This problem does not occur when time_strfrtime() gets
the current time itself since that codepath uses a
struct tm value directly from the libc localtime(),
leaving the tm_gmtoff field intact.

Fortunately the fix is almost trivial. A call to
mktime() will normalize the fields in the struct tm
value, including the tm_gmtoff field.  I
conditionalized the patch only on HAVE_MKTIME. I'm
pretty sure there's an autoconfigure test for tm_gmtoff
and it would probably be better to use that.
msg44159 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2003-07-09 22:36
Logged In: YES 
user_id=357491

So the problem is with the %z directive not reporting the proper 
timezone offset, correct?  If you look at http://www.python.org/
dev/doc/devel/lib/module-time.html , though, you will notice that 
%T is not supported as a directive for strftime or strptime nor has 
it ever been supported.  Same goes for %T although it works in 
your example.

Since the docs do not say that these directives are supported I am 
closing this patch since it would require altering both strftime and 
stprtime to support %z on all platforms which this patch does not.
msg44160 - (view) Author: Doug Quale (quale) Date: 2003-07-10 18:14
Logged In: YES 
user_id=812266

The problem isn't in strftime. The problem is in gettmargs() in 
timemodule.c.

Python assumes that broken down time tuples are in the local 
timezone. The gettmargs() routine in timemodule.c is bugged 
on GNU libc and possibly other BSD-inspired C libraries. 
gettmargs() is supposed to take a Python broken down time 
tuple and convert it to a C struct tm. The Python time tuple 
is assumed to be in the local time zone, so the struct tm 
should be in the local timezone also. In glibc, struct tm has 
timezone fields so each struct tm knows its own timezone. 
The gettmargs() routine never fills in these extra fields so it 
always creates a struct tm in GMT. The appropriate behavior 
would be to set tm_gmtoff to the local timezone offset. My 
patch fixes gettmargs() to create struct tm's in the local 
timezone for C libraries that have the tm_gmtoff field in struct 
tm.

As to the docs issue, the Python docs say that other formats 
may be supported than the ones listed. In reality, strftime() is 
passed to the underlying C library so the format codes 
supported are whatever the C library supports. The doc 
statement "Additional directives may be supported on certain 
platforms, but only the ones listed here have a meaning 
standardized by ANSI C" is wrong, or at least not up to date. 
C99 specifies several strftime format codes that are not listed 
including %z. I think Tim Smith also mentions this in a Python 
list posting from earlier this year.

In the Python time module, the docs say strftime('format') is 
the same as strftime('format', localtime()). This is simply 
broken right now on glibc as has been reported by more than 
one person:

>>> strftime('%z')
'-0500'
>>> strftime('%z', localtime())
'+0000'

This is wrong. Unsupported format specifiers do not have this 
effect, for example:

>>> strftime('%L')
'%L'
>>> strftime('%L', localtime())
'%L'

This behavior is correct.

A final note on the patch: I should have looked closer at the 
timemodule.c source. It already uses the appropriate #ifdef in 
other places.  Instead of #ifdef HAVE_MKTIME my patch 
should be conditionalized #ifdef HAVE_STRUCT_TM_TM_ZONE.

It's kind of amusing to write up this long of a justification for 
what is essentially a 3 line patch.
msg44161 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-07-11 23:52
Logged In: YES 
user_id=80475

Martin, can you diagnose whether this should be closed, 
applied, or deferred?
msg44162 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2003-08-09 13:00
Logged In: YES 
user_id=21627

The patch is wrong. It changes the behaviour of

time.asctime(time.gmtime(time.time()))

which it shouldn't do. The problem is real, though, and
might need to be solved by exposing the tm_gmtoff field
where available.
msg44163 - (view) Author: Doug Quale (quale) Date: 2003-08-09 17:16
Logged In: YES 
user_id=812266

Martin is right. The call to mktime() in my patch overwrites
the tm_isdst field. This field is in the Python time tuple
and is correctly set by the code immediately above. I put
the call to mktime in the wrong place. Instead of going at
the end of the routine it belongs immediately after the
memset used to zero the structure.

Sorry about this botch. I have attached a corrected patch.
msg44164 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2003-08-17 21:54
Logged In: YES 
user_id=80475

Can you attach a unittest that fails before the patch and 
succeeds afterward?
msg44165 - (view) Author: Doug Quale (quale) Date: 2003-08-21 16:57
Logged In: YES 
user_id=812266

I have attached a little unittest with two tests showing the
problem. I stole some code from Lib/test/test_time.py for
the test that checks behavior in the US Eastern timezone.
That test won't be run if tzset() isn't available, but this
is OK since the only libc's that will show this problem are
BSD-inspired and will have tzset().
msg44166 - (view) Author: Paul Boddie (pboddie) Date: 2007-02-24 00:02
I've just tried this patch against Python 2.4.4, changing the define tested to HAVE_TM_ZONE (in line with the autoconf test AC_STRUCT_TIMEZONE), but it didn't seem to produce the desired result (despite activating the new code in calls to gettmargs):

Python 2.4.4 (#1, Feb 23 2007, 12:37:26)
[GCC 3.3.5 (Debian 1:3.3.5-8ubuntu2.1)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> lt = time.localtime()
>>> time.strftime("%Y-%m-%d %H:%M:%S %z", lt)
'2007-02-24 00:45:23 +0000'

The %z output should be '+0100'. According to the mktime man page, the time zone should be set "as though mktime() called tzset()", but a simple C test program revealed that either the tm_gmtoff field remains unset or is set to zero (which is not appropriate on my system).

In other words, mktime does not miraculously restore the time zone information prior to the structure initialisation in gettmargs, at least on my system.
msg97987 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-18 03:02
Here's an updated version of the previous patches with the test included in test_time.py. The test fails on Linux before the timemodule.c file is changed, then it passes once applied.
msg103309 - (view) Author: Grant Bowman (grantbow) Date: 2010-04-16 10:24
Is there anything others like me can do to help get this fixed?
msg103321 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-04-16 13:12
Review the code, test the patch, confirm that it fixes the problem and doesn't break anything else, and report your results here.  That doesn't guarantee that it will get applied, but it definitely helps.
msg103369 - (view) Author: Paul Boddie (pboddie) Date: 2010-04-16 21:57
Well, this still doesn't work for me. I'm running Kubuntu 8.04 (libc6 package version 2.7-10ubuntu5) and reside in the CEST time zone, yet attempting to display the time zone always seems to give "+0000". Here are the failing tests, too:

======================================================================
FAIL: test_tm_gmtoff1 (__main__.TimeTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/test/test_time.py", line 225, in test_tm_gmtoff1
    time.strftime("%z"), time.strftime("%z", time.localtime()))
AssertionError: '+0200' != '+0000'

======================================================================
FAIL: test_tm_gmtoff2 (__main__.TimeTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/test/test_time.py", line 238, in test_tm_gmtoff2
    time.strftime("%z", time.localtime()), "+0000")
AssertionError: '+0000' == '+0000'
msg108731 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-26 16:12
I agree with Martin.  A proper fix would be to use tm_gmtoff explicitly where available and carry it in time.struct_time.

See issue1647654 and issue4086.

Interestingly, the issue does not show up on OSX, which being a BSD derivative does have tm_gmtoff in struct tm.

While I don't have an affected system to test the patch, it does not look right to me.  I would think mktime() should be called after PyArg_Parse, not before because tm_gmtoff may be different for different times.
msg108732 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-26 16:15
Another related issue is issue1667546.
msg138307 - (view) Author: Stephen White (Stephen.White) Date: 2011-06-14 10:15
Debian appear to have applied this patch, and it seems to be causing problems:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=593461
msg138340 - (view) Author: Stephen White (Stephen.White) Date: 2011-06-14 16:03
The patch, issue762963.diff, is broken.  It is calling mktime on a struct tm that is initialized to zeros.  This means that it should be filling in the missing fields based on their correct values for the date 1st Jan 1900, which is incorrect behaviour as the whole method should be choosing appropriate values based on the date provided by the user.

However in practice this call to mktime is effectively a no-op on 32bit systems.  The reason for this is:

The mktime(p) call is at the top of the method, straight after the memset(p, '\0', ...) call.  This means p->tm_year is zero.  According to the definition of struct tm a zero in the year field means 1900.

On a 32bit system the earliest date handled by libc is 2**31 seconds before the Epoch (1st Jan 1970);
>>> time.strftime("%Y-%m-%d %H:%M:%S %Z", time.localtime(-2**31))'1901-12-13 20:45:52 GMT'

So dates in the year 1900 cannot be handled by libc, and in this situation the mktime(p) call makes no attempt to normalise the provided data (or fill in missing values).

The situation is different on 64bit systems.  Here there is no problem with a much wider range of dates.  This means that dates during 1900 *are* handled by libc, and so it does attempt to normalise the data and fill in missing values.

For most of the fields in the structure whether or not mktime fills in or alters their value is of little consequence, as they're immediately overwritten by the call to PyArg_Parse.  However the contents of the tm_gmtoff & tm_zone fields are not overwritten.

If the mktime call does nothing (as on a 32bit system) then tm_zone remains NULL throughout.

If the mktime call does fill in missing values (as on 64bit systems) then tm_zone is set to the appropriate timezone for the zero time (the beginning of the year 1900).  In our case this is always "GMT", because the beginning of the year is in winter (when we use GMT).

If tm_zone is set when the structure is passed into strftime then it is honoured.  So if it has been set by mktime to be GMT then strftime will output GMT, regardless of the correct timezone string for the actual time provided.
msg138354 - (view) Author: Paul Boddie (pboddie) Date: 2011-06-14 22:05
I don't understand how this bug and its patches are still active. It's difficult for me to remember what I was doing in early 2007 when I started working on issue #1667546, but I can well imagine that it was in response to this and a number of related bugs. Looking at #1667546, it's clear that the work required to handle time zones is not at all as trivial as the patches attached to this issue appear to be.
History
Date User Action Args
2015-10-02 21:30:21belopolskysetstatus: open -> closed
superseder: Time zone-capable variant of time.localtime
resolution: out of date
2013-01-25 19:14:46brett.cannonsetnosy: - brett.cannon
2011-06-14 22:05:49pboddiesetmessages: + msg138354
2011-06-14 16:03:36Stephen.Whitesetmessages: + msg138340
2011-06-14 10:48:19jonsiddlesetnosy: + jonsiddle
2011-06-14 10:15:11Stephen.Whitesetnosy: + Stephen.White
messages: + msg138307
2010-06-26 16:15:29belopolskysetmessages: + msg108732
2010-06-26 16:12:08belopolskysetmessages: + msg108731
2010-06-26 03:20:27belopolskysetnosy: + belopolsky
2010-04-16 21:57:12pboddiesetmessages: + msg103369
2010-04-16 13:12:14r.david.murraysetnosy: + r.david.murray
messages: + msg103321
2010-04-16 10:24:32grantbowsetnosy: + grantbow
messages: + msg103309
2010-01-18 03:02:40brian.curtinsetfiles: + issue762963.diff

type: behavior
versions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.5
keywords: + needs review
nosy: + brian.curtin

messages: + msg97987
stage: patch review
2008-01-06 12:07:41christian.heimessetversions: + Python 2.6, Python 2.5
2003-06-30 02:18:54qualecreate