Summary:	ASTERISK-13421: [patch] zoneinfo caching causes incorrect time
Reporter:	James McCoy (jamessan)	Labels:
Date Opened:	2009-01-21 14:05:15.000-0600	Date Closed:	2009-02-25 13:41:10.000-0600
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20090127__bug14300.diff.txt ( 1) 20090224__bug14300.diff ( 2) localtime.diff
Description:	In stdtime/localtime.c, the information for time-zones are cached in a state struct.  This caching is keyed off the name of the zone passed into ast_tzset (using /etc/localtime if none is provided). This is problematic since it isn't uncommon for the localtime file to be a symlink to/copy of the actual zoneinfo file.  Therefore, if the target of the symlink is changed to affect a change in the system's timezone, Asterisk will not recognize the change until it is restarted. The attached patch uses realpath(3) to attempt expanding the zone to a full path and using that as the key instead.  This will ensure that if a path like "/etc/localtime" or "/usr/share/zoneinfo/localtime" is being used and is a symlink, changes in the target of the symlink will cause the zoneinfo to be updated. The limitations of this patch are: 1) Only notices changes when symlinks are used.  If "/etc/localtime" (or whatever the common file is) is a copy/hardlink instead, changes won't be noticed.  Making use of lstat(2) instead of realpath(3) may be an approach to consider. 2) It doesn't handle the condition where zone is passed to ast_tzset and is using a relative path and/or the zone contains a leading colon.  There's code in tzload that determines the absolute path which could be pulled out into a common function.  Feeding the results of that into realpath(3) would then handle the different formats that the zone could be specified in.
Comments:	By: Tilghman Lesher (tilghman) 2009-01-21 14:49:45.000-0600 I see your point, but I have a couple of comments: 1) Changing the timezone typically only occurs at system setup and never thereafter.  This would thus be extra code that isn't otherwise necessary. Therefore, I don't think this is appropriate for anything other than trunk. 2) The manpage for realpath(3) notes that it is broken by design, since it does not take the length of a buffer as a parameter, and thus is prone to buffer overflows.  readlink(2) is probably preferable. By: James McCoy (jamessan) 2009-01-21 15:31:51.000-0600 1) True, changing the timezone happens infrequently, but it also means that anything that uses Asterisk's time functions have the wrong time until Asterisk (or the system) is restarted. For appliances that are shipped with Asterisk and configured by the end-user, this can mean that Asterisk-based times/logs are skewed from the time on the rest of the system for a long time. Making the lookup only happen for uncached entries (when the zone passed to ast_tzset != NULL) would help but it needs to happen every time for /etc/localtime. 2) Ah, yes, readlink(2) would be better. If the general idea of the patch is acceptable, I can work on fixing the two limitations I mentioned in the original report as well as the realpath problem (although that may be fixed by addressing the first limitation). Hmm, maybe dispatching to the standard time functions when no specific zone has been given would be better?  As I understand it, the sole reason for this portion of the Asterisk code is to allow things like app_voicemail to have different timezones per-user and to specify the timezone in certain dialplan applications.  If a specific timezone hasn't been requested, is there a need to use these functions over the standard ones? By: Tilghman Lesher (tilghman) 2009-01-21 16:25:18.000-0600 I believe libc has exactly the same problem of caching, as this code is derived from the upstream source (US govt official time), so using those standard time functions would give exactly the same results. By: James McCoy (jamessan) 2009-01-21 16:38:32.000-0600 Other long-running processes on our system see the timezone change, so if libc does do caching it is detecting the file change. By: John Covert (jcovert) 2009-01-21 18:16:54.000-0600 >Changing the timezone typically only occurs at system >setup and never thereafter. You mean you don't run Asterisk in your laptop? I would love to have jamessan's patch.  It would mean that when I show up in Timbuktu (or Las Vegas) to teach an Asterisk class, my portable Asterisk system will have a correct view of the time, and can route pre-6am calls from the East Coast to something that won't wake me up. By: Tilghman Lesher (tilghman) 2009-01-26 17:24:46.000-0600 This would be fine, but I would suggest the use of the inotify(7) interface, instead.  This eliminates an extra hit of checking the modtime on every date/time access.  The stat(2) call is fairly costly, compared to other calls. By: Tilghman Lesher (tilghman) 2009-01-27 15:02:31.000-0600 Patch using inotify(7) interface uploaded against trunk. By: James McCoy (jamessan) 2009-02-10 14:41:30.000-0600 Sorry for the delay.  The non-inotify portion of the patch worked for me.  I can't test the inotify part in my environment since we're using an older glibc and I haven't had time to test a standalone build. By: James McCoy (jamessan) 2009-02-24 23:13:03.000-0600 Ok, finally got some time to test the inotify portion of the patch.  Updated patch attached to fix the issues I found: - Need to store the watch descriptor for the inotify events since the event name is only populated when a directory is being watched. - Need to specify IN_CLOSE_WRITE to notice when a files contents were changed but the inode remained the same.  IN_MODIFY didn't appear to be sufficient. - Need to pass the correct filename to add_notify With the above changes, patch worked with both poll and inotify interfaces. By: Digium Subversion (svnbot) 2009-02-25 13:25:02.000-0600 Repository: asterisk Revision: 178605 U   trunk/configure U   trunk/configure.ac U   trunk/include/asterisk/autoconfig.h.in U   trunk/main/stdtime/localtime.c ------------------------------------------------------------------------ r178605 | tilghman | 2009-02-25 13:25:02 -0600 (Wed, 25 Feb 2009) | 9 lines Use notification when timezone files change and re-scan then. (closes issue ASTERISK-13421) Reported by: jamessan Patches:       20090127__bug14300.diff.txt uploaded by tilghman (license 14)       20090224__bug14300.diff uploaded by jamessan (license 246) Tested by: jamessan Review: http://reviewboard.digium.com/r/136/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=178605 By: Digium Subversion (svnbot) 2009-02-25 13:41:09.000-0600 Repository: asterisk Revision: 178606 _U  branches/1.6.1/ ------------------------------------------------------------------------ r178606 | tilghman | 2009-02-25 13:41:09 -0600 (Wed, 25 Feb 2009) | 15 lines Blocked revisions 178605 via svnmerge ........  r178605 | tilghman | 2009-02-25 13:24:44 -0600 (Wed, 25 Feb 2009) | 9 lines    Use notification when timezone files change and re-scan then.  (closes issue ASTERISK-13421)   Reported by: jamessan   Patches:         20090127__bug14300.diff.txt uploaded by tilghman (license 14)         20090224__bug14300.diff uploaded by jamessan (license 246)   Tested by: jamessan   Review: http://reviewboard.digium.com/r/136/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=178606