Asterisk does its own TZ time formatting rather than using the C library functions. This seems to be so that it can watch the zoneinfo files for changes (TZ data updates are not very uncommon on GNU/Linux systems). To implement file watching there is cpp conditional code for Linux inotify, else for kqueue(2), else sleep-and-stat code. The file watch code runs in a thread.
There are a few bugs in the time formatting code. One in particular results from the fact that kqueue is used with open file descriptors and some non-fatal errors can cause a proc tzparse() to be called repeatedly, and tzparse() unconditionally causes a zoneinfo file (/usr/share/zoneinfo/posixrules) to be opened and added to the kqueue each time, and these descriptors are not associated the state structs that the code maintains. Needless to say, I got into this code when a call failed and I found Asterisk was out of files. Next problem is: why the non-fatal errors mentioned above that trigger the leak? Seems to be that a macro that removes list items from the head of a state list fails to NULL pointers when the last item is removed. There are a couple more misc. bugs addressed in the patch I am appending. The patch is against 11.15.0, and if requested I can post a patch against 11.7.0 (5.5 ports), or Asterisk svn trunk of a couple days ago. The patch might appear to be larger than it would seem if it were smaller than it is. It guards against reopening posixrules, adds {de,}allocators for the structs that hold descriptors for the queue, initializes the (list) empty head, and misc. -Ed --- main/stdtime/localtime.c.orig Thu Jan 29 15:05:25 2015 +++ main/stdtime/localtime.c Thu Jan 29 15:25:35 2015 @@ -253,6 +253,9 @@ int doextend)); static int tzparse P((const char * name, struct state * sp, int lastditch)); +/* struct state allocator with additional setup as needed */ +static struct state * sstate_alloc(void); +static void sstate_free(struct state *p); static AST_LIST_HEAD_STATIC(zonelist, state); #ifdef HAVE_NEWLOCALE @@ -282,6 +285,20 @@ #ifdef HAVE_INOTIFY static int inotify_fd = -1; +/* extra initialisation for sstate_alloc() */ +#define SP_INIT_EX(sp) do { \ + (sp)->wd[0] = -1; \ + (sp)->wd[1] = -1; \ +} while (0) +#if 0 /* untested: inotify users can try if they wish */ +#define SP_FREE_EX(sp) do { \ + if ((sp)->wd[0] > -1) { inotify_rm_watch(inotify_fd, (sp)->wd[0]); (sp)->wd[0] = -1; } \ + if ((sp)->wd[1] > -1) { inotify_rm_watch(inotify_fd, (sp)->wd[1]); (sp)->wd[1] = -1; } \ +} while (0) +#else /* this results in no change */ +#define SP_FREE_EX(sp) do {;} while (0) +#endif + static void *inotify_daemon(void *data) { struct { @@ -327,7 +344,7 @@ AST_LIST_TRAVERSE_SAFE_BEGIN(&zonelist, cur, list) { if (cur->wd[0] == buf.iev.wd || cur->wd[1] == buf.iev.wd) { AST_LIST_REMOVE_CURRENT(list); - ast_free(cur); + sstate_free(cur); break; } } @@ -376,14 +393,60 @@ #elif defined(HAVE_KQUEUE) static int queue_fd = -1; +/* + * static struct state *psx_sp and associated code will guard againt + * add_notify() called repeatedly for /usr/share/zoneinfo/posixrules + * and leaking a desciptor each time as a result of some errors + * (any code where tzparse() is called if tzload() fails -- + * tzparse() re-calls tzload() for /usr/share/zoneinfo/posixrules) + */ +static struct state *psx_sp = NULL; +/* + * kqueue_daemon_freestate() called from kqueue_daemon() -- + * replaces resource release code that was in line there + */ +static void kqueue_daemon_freestate(struct state *); + +/* collect EVFILT_VNODE fflags in macro; + * RENAME and LINK and TRUNCATE added */ +#ifdef NOTE_TRUNCATE +# define EVVN_NOTES_BITS \ + (NOTE_DELETE|NOTE_WRITE|NOTE_EXTEND|NOTE_REVOKE|NOTE_ATTRIB \ + |NOTE_RENAME|NOTE_LINK|NOTE_TRUNCATE) +#else +# define EVVN_NOTES_BITS \ + (NOTE_DELETE|NOTE_WRITE|NOTE_EXTEND|NOTE_REVOKE|NOTE_ATTRIB \ + |NOTE_RENAME|NOTE_LINK) +#endif + +/* extra initialisation for sstate_alloc() */ +#ifdef HAVE_O_SYMLINK +#define SP_INIT_EX(sp) do { \ + (sp)->fd = -1; \ + (sp)->fds = -1; \ +} while (0) +#define SP_FREE_EX(sp) do { \ + if ((sp)->fd > -1) { close((sp)->fd); (sp)->fd = -1; } \ + if ((sp)->fds > -1) { close((sp)->fds); (sp)->fds = -1; } \ +} while (0) +#else +#define SP_INIT_EX(sp) do { \ + (sp)->fd = -1; \ + (sp)->dir = NULL; \ +} while (0) +#define SP_FREE_EX(sp) do { \ + if ((sp)->fd > -1) { close((sp)->fd); (sp)->fd = -1; } \ + if ((sp)->dir != NULL) { closedir((sp)->dir); (sp)->dir = NULL; } \ +} while (0) +#endif + static void *kqueue_daemon(void *data) { struct kevent kev; struct state *sp; - struct timespec no_wait = { 0, 1 }; ast_mutex_lock(&initialization_lock); - if ((queue_fd = kqueue()) < 0) { + if (queue_fd < 0 && (queue_fd = kqueue()) < 0) { /* ast_log uses us to format messages, so if we called ast_log, we'd be * in for a nasty loop (seen already in testing) */ fprintf(stderr, "Unable to initialize kqueue(): %s\n", strerror(errno)); @@ -410,56 +473,106 @@ sp = kev.udata; - /*!\note - * If the file event fired, then the file was removed, so we'll need - * to reparse the entry. The directory event is a bit more - * interesting. Unfortunately, the queue doesn't contain information - * about the file that changed (only the directory itself), so unless - * we kept a record of the directory state before, it's not really - * possible to know what change occurred. But if we act paranoid and - * just purge the associated file, then it will get reparsed, and - * everything works fine. It may be more work, but it's a vast - * improvement over the alternative implementation, which is to stat - * the file repeatedly in what is essentially a busy loop. */ + /* see comment near psx_sp in add_notify() */ + if (sp == psx_sp) { + AST_LIST_LOCK(&zonelist); + psx_sp = NULL; + fprintf(stderr, + "kevent %#x on default TZ rules %s\n", + (unsigned int)kev.fflags, sp->name); + + kqueue_daemon_freestate(sp); + + while ((sp = AST_LIST_REMOVE_HEAD(&zonelist, list))) { + kqueue_daemon_freestate(sp); + } + + AST_LIST_HEAD_INIT_NOLOCK(&zonelist); + AST_LIST_UNLOCK(&zonelist); + } else { + AST_LIST_LOCK(&zonelist); + AST_LIST_REMOVE(&zonelist, sp, list); + kqueue_daemon_freestate(sp); + AST_LIST_UNLOCK(&zonelist); + } + + /* Just in case the signal was sent late */ AST_LIST_LOCK(&zonelist); - AST_LIST_REMOVE(&zonelist, sp, list); + ast_cond_broadcast(&initialization); AST_LIST_UNLOCK(&zonelist); + } + inotify_thread = AST_PTHREADT_NULL; + return NULL; +} + +static void kqueue_daemon_freestate(struct state *sp) +{ + struct kevent kev; + struct timespec no_wait = { 0, 1 }; + + /*!\note + * If the file event fired, then the file was removed, so we'll need + * to reparse the entry. The directory event is a bit more + * interesting. Unfortunately, the queue doesn't contain information + * about the file that changed (only the directory itself), so unless + * we kept a record of the directory state before, it's not really + * possible to know what change occurred. But if we act paranoid and + * just purge the associated file, then it will get reparsed, and + * everything works fine. It may be more work, but it's a vast + * improvement over the alternative implementation, which is to stat + * the file repeatedly in what is essentially a busy loop. */ + + if (sp->fd > -1) { /* If the directory event fired, remove the file event */ EV_SET(&kev, sp->fd, EVFILT_VNODE, EV_DELETE, 0, 0, NULL); kevent(queue_fd, &kev, 1, NULL, 0, &no_wait); - close(sp->fd); + } #ifdef HAVE_O_SYMLINK - if (sp->fds > -1) { - /* If the file event fired, remove the symlink event */ - EV_SET(&kev, sp->fds, EVFILT_VNODE, EV_DELETE, 0, 0, NULL); - kevent(queue_fd, &kev, 1, NULL, 0, &no_wait); - close(sp->fds); - } + if (sp->fds > -1) { + /* If the file event fired, remove the symlink event */ + EV_SET(&kev, sp->fds, EVFILT_VNODE, EV_DELETE, 0, 0, NULL); + kevent(queue_fd, &kev, 1, NULL, 0, &no_wait); + } #else - if (sp->dir) { - /* If the file event fired, remove the directory event */ - EV_SET(&kev, dirfd(sp->dir), EVFILT_VNODE, EV_DELETE, 0, 0, NULL); - kevent(queue_fd, &kev, 1, NULL, 0, &no_wait); - closedir(sp->dir); - } + if (sp->dir) { + /* If the file event fired, remove the directory event */ + EV_SET(&kev, dirfd(sp->dir), EVFILT_VNODE, EV_DELETE, 0, 0, NULL); + kevent(queue_fd, &kev, 1, NULL, 0, &no_wait); + } #endif - ast_free(sp); - /* Just in case the signal was sent late */ - AST_LIST_LOCK(&zonelist); - ast_cond_broadcast(&initialization); - AST_LIST_UNLOCK(&zonelist); - } + sstate_free(sp); } static void add_notify(struct state *sp, const char *path) { struct kevent kev; struct timespec no_wait = { 0, 1 }; - char watchdir[PATH_MAX + 1] = ""; + char watchdir[PATH_MAX + 1] = ""; + /* this block guards against a result of a failure return + * from tzload() in several procs, in which tzparse() is then + * called, and that calls tzload() again, unconditionally, + * for TZDEFRULES (e.g., /usr/share/zoneinfo/posixrules); + * if errors repeat, each call open(2)s and watches and a + * descriptor leak is the result. So, make sure only one + * 'watch' is used (and if it gets an event, all state are + * freed). + */ + if (! strcmp(path, TZDEFRULES)) { + if (psx_sp != NULL || + (psx_sp = sstate_alloc()) == NULL) { + return; + } + ast_copy_string(psx_sp->name, path, + sizeof(psx_sp->name)); + sp = psx_sp; + } else if (sp->fd != -1) { + return; + } + if (inotify_thread == AST_PTHREADT_NULL) { ast_cond_init(&initialization, NULL); ast_mutex_init(&initialization_lock); @@ -482,7 +595,8 @@ | O_EVTONLY # endif )) >= 0) { - EV_SET(&kev, sp->fds, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_ONESHOT, NOTE_WRITE | NOTE_EXTEND | NOTE_DELETE | NOTE_REVOKE | NOTE_ATTRIB, 0, sp); + EV_SET(&kev, sp->fds, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_ONESHOT, EVVN_NOTES_BITS, 0, sp); + errno = 0; if (kevent(queue_fd, &kev, 1, NULL, 0, &no_wait) < 0 && errno != 0) { /* According to the API docs, we may get -1 return value, due to the * NULL space for a returned event, but errno should be 0 unless @@ -518,7 +632,8 @@ * Likewise, there's no potential leak of a descriptor. */ EV_SET(&kev, dirfd(sp->dir), EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_ONESHOT, - NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_REVOKE | NOTE_ATTRIB, 0, sp); + EVVN_NOTES_BITS, 0, sp); + errno = 0; if (kevent(queue_fd, &kev, 1, NULL, 0, &no_wait) < 0 && errno != 0) { fprintf(stderr, "Unable to watch '%s': %s\n", watchdir, strerror(errno)); closedir(sp->dir); @@ -538,7 +653,8 @@ return; } - EV_SET(&kev, sp->fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_ONESHOT, NOTE_WRITE | NOTE_EXTEND | NOTE_DELETE | NOTE_REVOKE | NOTE_ATTRIB, 0, sp); + EV_SET(&kev, sp->fd, EVFILT_VNODE, EV_ADD | EV_ENABLE | EV_ONESHOT, EVVN_NOTES_BITS, 0, sp); + errno = 0; if (kevent(queue_fd, &kev, 1, NULL, 0, &no_wait) < 0 && errno != 0) { /* According to the API docs, we may get -1 return value, due to the * NULL space for a returned event, but errno should be 0 unless @@ -550,6 +666,11 @@ } } #else + +/* extra initialisation for sstate_alloc() */ +#define SP_INIT_EX(sp) do {;} while (0) +#define SP_FREE_EX(sp) do {;} while (0) + static void *notify_daemon(void *data) { struct stat st, lst; @@ -589,7 +710,7 @@ ast_log(LOG_NOTICE, "Removing cached TZ entry '%s' because underlying file changed.\n", name); } AST_LIST_REMOVE_CURRENT(list); - ast_free(cur); + sstate_free(cur); continue; } } @@ -623,6 +744,28 @@ } #endif +/* + * struct state allocator with additional setup as needed + */ +static struct state *sstate_alloc(void) +{ + struct state *p; + + if ((p = ast_calloc(1, sizeof(*p))) != NULL) { + SP_INIT_EX(p); + } + + return p; +} + +static void sstate_free(struct state *p) +{ + if (p != NULL) { + SP_FREE_EX(p); + ast_free(p); + } +} + void ast_localtime_wakeup_monitor(struct ast_test *info) { if (inotify_thread != AST_PTHREADT_NULL) { @@ -737,7 +880,9 @@ } } nread = read(fid, u.buf, sizeof u.buf); - if (close(fid) < 0 || nread <= 0) + /* comp was nread <= 0; + use nread < sizeof u.tzhead against unexpected short files */ + if (close(fid) < 0 || nread < sizeof u.tzhead) return -1; for (stored = 4; stored <= 8; stored *= 2) { int ttisstdcnt; @@ -864,6 +1009,11 @@ nread -= p - u.buf; for (i = 0; i < nread; ++i) u.buf[i] = p[i]; + /* next loop iter. will assume at least + sizeof(struct tzhead) bytes */ + if (nread < sizeof(u.tzhead)) { + break; + } /* ** If this is a narrow integer time_t system, we're done. */ @@ -1443,8 +1593,9 @@ AST_LIST_LOCK(&zonelist); while ((sp = AST_LIST_REMOVE_HEAD(&zonelist, list))) { - ast_free(sp); + sstate_free(sp); } + AST_LIST_HEAD_INIT_NOLOCK(&zonelist); AST_LIST_UNLOCK(&zonelist); } @@ -1472,7 +1623,7 @@ } AST_LIST_UNLOCK(&zonelist); - if (!(sp = ast_calloc(1, sizeof *sp))) + if (!(sp = sstate_alloc())) return NULL; if (tzload(zone, sp, TRUE) != 0) { @@ -1724,8 +1875,10 @@ } if (!sp) { - if (!(sp = (struct state *) ast_calloc(1, sizeof *sp))) + if (!(sp = sstate_alloc())) { + AST_LIST_UNLOCK(&zonelist); return NULL; + } gmtload(sp); AST_LIST_INSERT_TAIL(&zonelist, sp, list); } @@ -2304,6 +2457,7 @@ long fraction; const char *prevlocale; + buf[0] = '\0';/* Ensure the buffer is initialized. */ if (!format) { return -1; }