Re: thread-safety: gmtime_r(), localtime_r() - Mailing list pgsql-hackers

On 04.07.24 18:36, Heikki Linnakangas wrote:
> The Linux man page for localtime_r() says:
> 
>> According to POSIX.1-2001, localtime() is required to behave as
>> though tzset(3) was called, while localtime_r() does not have  this
>> requirement.   For  portable  code,  tzset(3) should be called before
>> localtime_r().
> 
> It's not clear to me what happens if tzset() has not been called and the 
> localtime_r() implementation does not call it either. I guess some 
> implementation default timezone is used.
> 
> In the libpq traces, I don't think we care much. In ecpg, I'm not sure 
> what the impact is if the application has not previously called tzset(). 
> I'd suggest that we just document that an ecpg application should call 
> tzset() before calling the functions that are sensitive to local 
> timezone setting.

I have been studying this question.  It appears that various libc 
implementers have been equally puzzled by this; there are various 
comments like "it's unclear what POSIX wants here" in the sources.  (I 
have checked glibc, FreeBSD, and Solaris.)

Consider if a program calls localtime() or localtime_r() twice:

     localtime(...);
     ...
     localtime(...);

or

     localtime_r(...);
     ...
     localtime_r(...);

The question here is, how many times does this effectively (internally) 
call tzset().  There are three possible answers: 0, 1, or 2.

For localtime(), the answer is clear.  localtime() is required to call 
tzset() every time, so the answer is 2.

For localtime_r(), it's unclear.  What you are wondering, and I have 
been wondering, is whether the answer is 0 or non-zero (and possibly, if 
it's 0, will these calls misbehave badly).  What the libc implementers 
are wondering is whether the answer is 1 or 2.  The implementations 
whose source code I have checked think it should be 1.  They never 
consider that it could be 0 and it's okay to misbehave.

Where this difference appears it practice would be something like

     setenv("TZ", "foo");
     localtime(...);  // uses TZ foo
     setenv("TZ", "bar");
     localtime(...);  // uses TZ bar

versus

     setenv("TZ", "foo");
     localtime_r(...);  // uses TZ foo if first call in program
     setenv("TZ", "bar");
     localtime_r(...);  // probably does not use new TZ

If you want the second case to pick up the changed TZ setting, you must 
explicitly call tzset() to be sure.

I think, considering this, the proposed patch should be okay.  At least, 
the libraries are not going to misbehave if tzset() hasn't been called 
explicitly.  It will be called internally the first time it's needed.  I 
don't think we need to cater to cases like my example where the 
application changes the TZ environment variable but neglects to call 
tzset() itself.


 >> There is one affected call in the backend.  Most of the backend
 >> otherwise uses the custom functions pg_gmtime() and pg_localtime(),
 >> which are implemented differently.
 >
 > Do we need to call tzset(3) somewhere in backend startup? Or could we
 > replace that localtime() call in the backend with pg_localtime()?

Let's look at what this code actually does.  It just takes the current 
time and then loops through all possible weekdays and months to collect 
and cache the localized names.  The current time or time zone doesn't 
actually matter for this, we just need to fill in the struct tm a bit 
for strftime() to be happy.  We could probably replace this with 
gmtime() to avoid the question about time zone state.  (We probably 
don't even need to call time() beforehand, we could just use time zero. 
But the comment says "We use times close to current time as data for 
strftime().", which is probably prudent.)  (Since we are caching the 
results for the session, we're already not dealing with the hilarious 
hypothetical situation where the weekday and month names depend on the 
actual time, in case that is a concern.)