[tz] [PROPOSED PATCH] Make the library thread-safe if THREAD_SAFE is defined.

Paul Eggert eggert at cs.ucla.edu
Sun Aug 17 20:36:45 UTC 2014


* localtime.c [THREAD_SAFE]: Include pthread.h.
(VOLATILE): New macro.
(locallock) [THREAD_SAFE]: New static var.
(lock, unlock): New functions.
(lcl_is_set, gmt_is_set): Now VOLATILE.
(tzsetwall): Move cleaned-up guts to new function tzsetwall_unlocked,
for which this is now merely a locking wrapper.
(tzset): Similarly, for new function tzset_unlocked.
(localtime_tzset): New function, which does proper locking.
(localtime, localtime_r): Use it.
(gmtsub): Do not worry about initializing gmtptr, as that's now
the caller's responsibility.
(gmtime): Reimplement in terms of gmtime_r.
(timegm): Reimplement in terms of timeoff.
(gmtime_r, offtime, mktime, timeoff, time2posix, posix2time):
Lock at start and unlock at end.
* Makefile, NEWS: Document this.
---
 Makefile    |   3 +
 NEWS        |   4 +
 localtime.c | 249 ++++++++++++++++++++++++++++++++++++++++--------------------
 3 files changed, 173 insertions(+), 83 deletions(-)

diff --git a/Makefile b/Makefile
index 5bbd7e7..3d24c2a 100644
--- a/Makefile
+++ b/Makefile
@@ -123,6 +123,9 @@ LDLIBS=
 #  -DNO_RUN_TIME_WARNINGS_ABOUT_YEAR_2000_PROBLEMS_THANK_YOU=1
 #	if you do not want run time warnings about formats that may cause
 #	year 2000 grief
+#  -DTHREAD_SAFE=1 to make localtime.c thread-safe, as POSIX requires;
+#	not needed by the main-program tz code, which is single-threaded.
+#	Append other compiler flags as needed, e.g., -pthread on GNU/Linux.
 #  -Dtime_tz=\"T\" to use T as the time_t type, rather than the system time_t
 #  -DTZ_DOMAIN=\"foo\" to use "foo" for gettext domain name; default is "tz"
 #  -DTZ_DOMAINDIR=\"/path\" to use "/path" for gettext directory;
diff --git a/NEWS b/NEWS
index 28974c9..1a78041 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,10 @@ Unreleased, experimental changes
 
   Changes affecting code
 
+    The tz library is now thread-safe if compiled with THREAD_SAFE defined.
+    Although not needed for tz's own applications, which are single-threaded,
+    this supports POSIX better if the tz library is used in multithreaded apps.
+
     tzselect -c now uses a hybrid distance measure that works better
     in Africa.  (Thanks to Alan Barrett for noting the problem.)
 
diff --git a/localtime.c b/localtime.c
index 551b5a2..f499583 100644
--- a/localtime.c
+++ b/localtime.c
@@ -16,6 +16,18 @@
 #include "tzfile.h"
 #include "fcntl.h"
 
+#if THREAD_SAFE
+# include <pthread.h>
+# define VOLATILE volatile
+static pthread_mutex_t locallock = PTHREAD_MUTEX_INITIALIZER;
+static int lock(void) { return pthread_mutex_lock(&locallock); }
+static void unlock(void) { pthread_mutex_unlock(&locallock); }
+#else
+# define VOLATILE
+static int lock(void) { return 0; }
+static void unlock(void) { }
+#endif
+
 #ifndef TZ_ABBR_MAX_LEN
 #define TZ_ABBR_MAX_LEN	16
 #endif /* !defined TZ_ABBR_MAX_LEN */
@@ -154,8 +166,8 @@ static struct state	gmtmem;
 #endif /* !defined TZ_STRLEN_MAX */
 
 static char		lcl_TZname[TZ_STRLEN_MAX + 1];
-static int		lcl_is_set;
-static int		gmt_is_set;
+static int VOLATILE	lcl_is_set;
+static int VOLATILE	gmt_is_set;
 
 char *			tzname[2] = {
 	(char *) wildabbr,
@@ -1139,6 +1151,21 @@ gmtload(struct state *const sp)
 		(void) tzparse(gmt, sp, TRUE);
 }
 
+static void
+tzsetwall_unlocked(void)
+{
+  if (lcl_is_set < 0)
+    return;
+#ifdef ALL_STATE
+  if (! lclptr)
+    lclptr = malloc(sizeof *lclptr);
+#endif
+  if (lclptr && tzload(NULL, lclptr, TRUE) != 0)
+    gmtload(lclptr);
+  settzname();
+  lcl_is_set = -1;
+}
+
 #ifndef STD_INSPIRED
 /*
 ** A non-static declaration of tzsetwall in a system header file
@@ -1149,65 +1176,71 @@ static
 void
 tzsetwall(void)
 {
-	if (lcl_is_set < 0)
-		return;
-	lcl_is_set = -1;
+  if (lock() != 0)
+    return;
+  tzsetwall_unlocked();
+  unlock();
+}
 
+static void
+tzset_unlocked(void)
+{
+  int shortname;
+  register char const *name = getenv("TZ");
+
+  if (!name) {
+    tzsetwall_unlocked();
+    return;
+  }
+  shortname = strlen(name) < sizeof lcl_TZname;
+  if (0 < lcl_is_set && strcmp(lcl_TZname, name) == 0)
+    return;
+  if (shortname)
+    strcpy(lcl_TZname, name);
 #ifdef ALL_STATE
-	if (lclptr == NULL) {
-		lclptr = malloc(sizeof *lclptr);
-		if (lclptr == NULL) {
-			settzname();	/* all we can do */
-			return;
-		}
-	}
+  if (! lclptr)
+    lclptr = malloc(sizeof *lclptr);
 #endif /* defined ALL_STATE */
-	if (tzload(NULL, lclptr, TRUE) != 0)
-		gmtload(lclptr);
-	settzname();
+  if (lclptr) {
+    if (*name == '\0') {
+      /*
+      ** User wants it fast rather than right.
+      */
+      lclptr->leapcnt = 0;		/* so, we're off a little */
+      lclptr->timecnt = 0;
+      lclptr->typecnt = 0;
+      lclptr->ttis[0].tt_isdst = 0;
+      lclptr->ttis[0].tt_gmtoff = 0;
+      lclptr->ttis[0].tt_abbrind = 0;
+      strcpy(lclptr->chars, gmt);
+    } else if (tzload(name, lclptr, TRUE) != 0)
+      if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0)
+	gmtload(lclptr);
+  }
+  settzname();
+  lcl_is_set = shortname;
 }
 
 void
 tzset(void)
 {
-	register const char *	name;
-
-	name = getenv("TZ");
-	if (name == NULL) {
-		tzsetwall();
-		return;
-	}
-
-	if (lcl_is_set > 0 && strcmp(lcl_TZname, name) == 0)
-		return;
-	lcl_is_set = strlen(name) < sizeof lcl_TZname;
-	if (lcl_is_set)
-		(void) strcpy(lcl_TZname, name);
+  if (lock() != 0)
+    return;
+  tzset_unlocked();
+  unlock();
+}
 
+static void
+gmtcheck(void)
+{
+  if (gmt_is_set)
+    return;
 #ifdef ALL_STATE
-	if (lclptr == NULL) {
-		lclptr = malloc(sizeof *lclptr);
-		if (lclptr == NULL) {
-			settzname();	/* all we can do */
-			return;
-		}
-	}
-#endif /* defined ALL_STATE */
-	if (*name == '\0') {
-		/*
-		** User wants it fast rather than right.
-		*/
-		lclptr->leapcnt = 0;		/* so, we're off a little */
-		lclptr->timecnt = 0;
-		lclptr->typecnt = 0;
-		lclptr->ttis[0].tt_isdst = 0;
-		lclptr->ttis[0].tt_gmtoff = 0;
-		lclptr->ttis[0].tt_abbrind = 0;
-		(void) strcpy(lclptr->chars, gmt);
-	} else if (tzload(name, lclptr, TRUE) != 0)
-		if (name[0] == ':' || tzparse(name, lclptr, FALSE) != 0)
-			(void) gmtload(lclptr);
-	settzname();
+  gmtptr = malloc(sizeof *gmtptr);
+#endif
+  if (gmtptr)
+    gmtload(gmtptr);
+  gmt_is_set = TRUE;
 }
 
 /*
@@ -1296,11 +1329,25 @@ localsub(const time_t *const timep, const int_fast32_t offset,
 	return result;
 }
 
+static struct tm *
+localtime_tzset(time_t const *timep, struct tm *tmp, int skip_tzset)
+{
+  int err = lock();
+  if (err) {
+    errno = err;
+    return NULL;
+  }
+  if (!skip_tzset)
+    tzset_unlocked();
+  tmp = localsub(timep, 0, tmp);
+  unlock();
+  return tmp;
+}
+
 struct tm *
 localtime(const time_t *const timep)
 {
-	tzset();
-	return localsub(timep, 0L, &tm);
+  return localtime_tzset(timep, &tm, 0);
 }
 
 /*
@@ -1310,7 +1357,7 @@ localtime(const time_t *const timep)
 struct tm *
 localtime_r(const time_t *const timep, struct tm *tmp)
 {
-	return localsub(timep, 0L, tmp);
+  return localtime_tzset(timep, tmp, lcl_is_set);
 }
 
 /*
@@ -1323,16 +1370,6 @@ gmtsub(const time_t *const timep, const int_fast32_t offset,
 {
 	register struct tm *	result;
 
-	if (!gmt_is_set) {
-#ifdef ALL_STATE
-		gmtptr = malloc(sizeof *gmtptr);
-		gmt_is_set = gmtptr != NULL;
-#else
-		gmt_is_set = TRUE;
-#endif /* defined ALL_STATE */
-		if (gmt_is_set)
-			gmtload(gmtptr);
-	}
 	result = timesub(timep, offset, gmtptr, tmp);
 #ifdef TM_ZONE
 	/*
@@ -1348,7 +1385,7 @@ gmtsub(const time_t *const timep, const int_fast32_t offset,
 struct tm *
 gmtime(const time_t *const timep)
 {
-	return gmtsub(timep, 0L, &tm);
+  return gmtime_r(timep, &tm);
 }
 
 /*
@@ -1358,7 +1395,15 @@ gmtime(const time_t *const timep)
 struct tm *
 gmtime_r(const time_t *const timep, struct tm *tmp)
 {
-	return gmtsub(timep, 0L, tmp);
+  int err = lock();
+  if (err) {
+    errno = err;
+    return NULL;
+  }
+  gmtcheck();
+  tmp = gmtsub(timep, 0, tmp);
+  unlock();
+  return tmp;
 }
 
 #ifdef STD_INSPIRED
@@ -1366,7 +1411,16 @@ gmtime_r(const time_t *const timep, struct tm *tmp)
 struct tm *
 offtime(const time_t *const timep, const long offset)
 {
-	return gmtsub(timep, offset, &tm);
+  struct tm *tmp;
+  int err = lock();
+  if (err) {
+    errno = err;
+    return NULL;
+  }
+  gmtcheck();
+  tmp = gmtsub(timep, offset, &tm);
+  unlock();
+  return tmp;
 }
 
 #endif /* defined STD_INSPIRED */
@@ -1901,8 +1955,16 @@ time1(struct tm *const tmp,
 time_t
 mktime(struct tm *const tmp)
 {
-	tzset();
-	return time1(tmp, localsub, 0L);
+  time_t t;
+  int err = lock();
+  if (err) {
+    errno = err;
+    return -1;
+  }
+  tzset_unlocked();
+  t = time1(tmp, localsub, 0);
+  unlock();
+  return t;
 }
 
 #ifdef STD_INSPIRED
@@ -1918,17 +1980,25 @@ timelocal(struct tm *const tmp)
 time_t
 timegm(struct tm *const tmp)
 {
-	if (tmp != NULL)
-		tmp->tm_isdst = 0;
-	return time1(tmp, gmtsub, 0L);
+  return timeoff(tmp, 0);
 }
 
 time_t
 timeoff(struct tm *const tmp, const long offset)
 {
-	if (tmp != NULL)
-		tmp->tm_isdst = 0;
-	return time1(tmp, gmtsub, offset);
+  time_t t;
+  int err;
+  if (tmp)
+    tmp->tm_isdst = 0;
+  err = lock();
+  if (err) {
+    errno = err;
+    return -1;
+  }
+  gmtcheck();
+  t = time1(tmp, gmtsub, offset);
+  unlock();
+  return t;
 }
 
 #endif /* defined STD_INSPIRED */
@@ -1986,8 +2056,17 @@ leapcorr(time_t *timep)
 time_t
 time2posix(time_t t)
 {
-	tzset();
-	return t - leapcorr(&t);
+  time_t p;
+  int err = lock();
+  if (err) {
+    errno = err;
+    return -1;
+  }
+  if (!lcl_is_set)
+    tzset_unlocked();
+  p = t - leapcorr(&t);
+  unlock();
+  return p;
 }
 
 time_t
@@ -1995,8 +2074,13 @@ posix2time(time_t t)
 {
 	time_t	x;
 	time_t	y;
-
-	tzset();
+	int err = lock();
+	if (err) {
+	  errno = err;
+	  return -1;
+	}
+	if (!lcl_is_set)
+	  tzset_unlocked();
 	/*
 	** For a positive leap second hit, the result
 	** is not unique. For a negative leap second
@@ -2010,16 +2094,15 @@ posix2time(time_t t)
 			x++;
 			y = x - leapcorr(&x);
 		} while (y < t);
-		if (t != y)
-			return x - 1;
+		x -= y != t;
 	} else if (y > t) {
 		do {
 			--x;
 			y = x - leapcorr(&x);
 		} while (y > t);
-		if (t != y)
-			return x + 1;
+		x += y != t;
 	}
+	unlock();
 	return x;
 }
 
-- 
1.9.1



More information about the tz mailing list