library: ensure thread safety for all static variables

Even though we we had to abandon the master branch top
multi-thread effort and even though the newlib version
of a multi-threaded top provides no real benefit, that
whole exercise was not wasted. Rather, it has revealed
some deficiencies in our library which this addresses.

If two or more threads in the same address space tried
to access the same api simultaneously, there is a good
chance some function-local static variables will yield
some of those renowned unpredictable results. So, this
patch protects them with the '__thread' storage class.

Reference(s):
https://www.freelists.org/post/procps/a-few-more-patches,7

Signed-off-by: Jim Warner <james.warner@comcast.net>

diff --git a/proc/devname.c b/proc/devname.c
index 5e2f2594246ecc323ab171f30c2fbf7f8ea35b93..325877f964550b9e9facc80767a3275086d33d5b 100644 (file)
--- a/proc/devname.c
+++ b/proc/devname.c
@@ -325,7 +325,7 @@ static int ctty_name(char *restrict const buf, int pid) {
 
 /* number --> name */
 unsigned dev_to_tty(char *restrict ret, unsigned chop, dev_t dev_t_dev, int pid, unsigned int flags) {
-  static char buf[TTY_NAME_SIZE];
+  static __thread char buf[TTY_NAME_SIZE];
   char *restrict tmp = buf;
   unsigned dev = dev_t_dev;
   unsigned i = 0;

diff --git a/proc/escape.c b/proc/escape.c
index 6c903a3216787cd69804982e1f873039a86d4018..3f5d3422d2ae3cc1191c261353b7a0ac9a8273fd 100644 (file)
--- a/proc/escape.c
+++ b/proc/escape.c
@@ -91,7 +91,7 @@ static inline void esc_ctl (unsigned char *str, int len) {
 }
 
 int escape_str (unsigned char *dst, const unsigned char *src, int bufsize) {
-   static int utf_sw = 0;
+   static __thread int utf_sw = 0;
    int n;
 
    if (utf_sw == 0) {

diff --git a/proc/meminfo.c b/proc/meminfo.c
index 998858ca9058b810cba0629a6f82e57d230812a7..705e512223338c0ace198b818adcb0e50c4cde5e 100644 (file)
--- a/proc/meminfo.c
+++ b/proc/meminfo.c
@@ -669,7 +669,7 @@ static int meminfo_read_failed (
     head = buf;
 
     for (;;) {
-        static ENTRY e;      // just to keep coverity off our backs (e.data)
+        static __thread ENTRY e;  // keep coverity off our backs (e.data)
         ENTRY *ep;
 
         if (!(tail = strchr(head, ':')))
@@ -882,7 +882,7 @@ PROCPS_EXPORT struct meminfo_result *procps_meminfo_get (
         struct meminfo_info *info,
         enum meminfo_item item)
 {
-    static time_t sav_secs;
+    static __thread time_t sav_secs;
     time_t cur_secs;
 
     errno = EINVAL;

diff --git a/proc/pids.c b/proc/pids.c
index d7897c5efe92e3fe5659201fee2a29910b432c3d..778f2a90a1a419b7b2b65cbbff7fd6dcae3a7d12 100644 (file)
--- a/proc/pids.c
+++ b/proc/pids.c
@@ -1123,7 +1123,7 @@ static int pids_stacks_fetch (
  #define n_alloc  info->fetch.n_alloc
  #define n_inuse  info->fetch.n_inuse
  #define n_saved  info->fetch.n_alloc_save
-    static proc_t task;    // static for initial zeroes + later dynamic free(s)
+    static __thread proc_t task;  // static for initial 0's + later free(s)
     struct stacks_extent *ext;
 
     // initialize stuff -----------------------------------
@@ -1367,7 +1367,7 @@ PROCPS_EXPORT struct pids_stack *fatal_proc_unmounted (
         struct pids_info *info,
         int return_self)
 {
-    static proc_t self;
+    static __thread proc_t self;
     struct stacks_extent *ext;
 
     /* this is very likely the *only* newlib function where the
@@ -1404,7 +1404,7 @@ PROCPS_EXPORT struct pids_stack *procps_pids_get (
         struct pids_info *info,
         enum pids_fetch_type which)
 {
-    static proc_t task;    // static for initial zeroes + later dynamic free(s)
+    static __thread proc_t task;  // static for initial 0's + later free(s)
 
     errno = EINVAL;
     if (info == NULL)

diff --git a/proc/readproc.c b/proc/readproc.c
index 6c986266d40931eda702cc64fd448be5aca1f5a4..edcd016bc1a975823e156da2207146273917769b 100644 (file)
--- a/proc/readproc.c
+++ b/proc/readproc.c
@@ -965,7 +965,7 @@ static int fill_environ_cvt (const char *directory, proc_t *restrict p) {
     // tracking all names already seen thus avoiding the overhead of repeating
     // malloc() and free() calls.
 static char *lxc_containers (const char *path) {
-    static struct utlbuf_s ub = { NULL, 0 };   // util buffer for whole cgroup
+    static __thread struct utlbuf_s ub = { NULL, 0 };   // util buffer for whole cgroup
     static char lxc_none[] = "-";
     static char lxc_oops[] = "?";              // used when memory alloc fails
     /*
@@ -995,7 +995,7 @@ static char *lxc_containers (const char *path) {
         if ((p1 = strstr(ub.buf, (delim = lxc_delm1)))
         || ((p1 = strstr(ub.buf, (delim = lxc_delm2)))
         || ((p1 = strstr(ub.buf, (delim = lxc_delm3)))))) {
-            static struct lxc_ele {
+            static __thread struct lxc_ele {
                 struct lxc_ele *next;
                 char *name;
             } *anchor = NULL;
@@ -1112,8 +1112,8 @@ static void autogroup_fill (const char *path, proc_t *p) {
 // The pid (tgid? tid?) is already in p, and a path to it in path, with some
 // room to spare.
 static proc_t *simple_readproc(PROCTAB *restrict const PT, proc_t *restrict const p) {
-    static struct utlbuf_s ub = { NULL, 0 };    // buf for stat,statm,status
-    static struct stat sb;     // stat() buffer
+    static __thread struct utlbuf_s ub = { NULL, 0 };    // buf for stat,statm,status
+    static __thread struct stat sb;     // stat() buffer
     char *restrict const path = PT->path;
     unsigned flags = PT->flags;
     int rc = 0;
@@ -1235,8 +1235,8 @@ next_proc:
 // t is the POSIX thread  (task group member, generally not the leader)
 // path is a path to the task, with some room to spare.
 static proc_t *simple_readtask(PROCTAB *restrict const PT, proc_t *restrict const t, char *restrict const path) {
-    static struct utlbuf_s ub = { NULL, 0 };    // buf for stat,statm,status
-    static struct stat sb;     // stat() buffer
+    static __thread struct utlbuf_s ub = { NULL, 0 };    // buf for stat,statm,status
+    static __thread struct stat sb;     // stat() buffer
     unsigned flags = PT->flags;
     int rc = 0;
 
@@ -1356,7 +1356,7 @@ next_task:
 // This finds processes in /proc in the traditional way.
 // Return non-zero on success.
 static int simple_nextpid(PROCTAB *restrict const PT, proc_t *restrict const p) {
-  static struct dirent *ent;            /* dirent handle */
+  static __thread struct dirent *ent;   /* dirent handle */
   char *restrict const path = PT->path;
   for (;;) {
     ent = readdir(PT->procfs);
@@ -1374,7 +1374,7 @@ static int simple_nextpid(PROCTAB *restrict const PT, proc_t *restrict const p)
 // This finds tasks in /proc/*/task/ in the traditional way.
 // Return non-zero on success.
 static int simple_nexttid(PROCTAB *restrict const PT, const proc_t *restrict const p, proc_t *restrict const t, char *restrict const path) {
-  static struct dirent *ent;            /* dirent handle */
+  static __thread struct dirent *ent;   /* dirent handle */
   if(PT->taskdir_user != p->tgid){
     if(PT->taskdir){
       closedir(PT->taskdir);
@@ -1402,7 +1402,7 @@ static int simple_nexttid(PROCTAB *restrict const PT, const proc_t *restrict con
 // This "finds" processes in a list that was given to openproc().
 // Return non-zero on success. (tgid is a real headache)
 static int listed_nextpid (PROCTAB *PT, proc_t *p) {
-  static struct utlbuf_s ub = { NULL, 0 };
+  static __thread struct utlbuf_s ub = { NULL, 0 };
   pid_t pid = *(PT->pids)++;
   char *path = PT->path;
 
@@ -1461,9 +1461,9 @@ out:
 // the next unique process or task available.  If no more are available,
 // return a null pointer (boolean false).
 proc_t *readeither (PROCTAB *restrict const PT, proc_t *restrict x) {
-    static proc_t skel_p;    // skeleton proc_t, only uses tid + tgid
-    static proc_t *new_p;    // for process/task transitions
-    static int canary, leader;
+    static __thread proc_t skel_p;    // skeleton proc_t, only uses tid + tgid
+    static __thread proc_t *new_p;    // for process/task transitions
+    static __thread int canary, leader;
     char path[PROCPATHLEN];
     proc_t *ret;
 
@@ -1511,7 +1511,7 @@ end_procs:
 PROCTAB *openproc(unsigned flags, ...) {
     va_list ap;
     struct stat sbuf;
-    static int did_stat;
+    static __thread int did_stat;
     PROCTAB *PT = calloc(1, sizeof(PROCTAB));
 
     if (!PT)

diff --git a/proc/slabinfo.c b/proc/slabinfo.c
index 4a6d7ed56b9edb0409b4aa92b5ee19b3393b4ee7..59e4000eee624ab29c362c21a84db9c31f90a19c 100644 (file)
--- a/proc/slabinfo.c
+++ b/proc/slabinfo.c
@@ -839,7 +839,7 @@ PROCPS_EXPORT struct slabinfo_result *procps_slabinfo_get (
         struct slabinfo_info *info,
         enum slabinfo_item item)
 {
-    static time_t sav_secs;
+    static __thread time_t sav_secs;
     time_t cur_secs;
 
     errno = EINVAL;

diff --git a/proc/stat.c b/proc/stat.c
index 953deee599b912eb46d3c9595b6acb594d1221cc..e2ec4a3acc5e2327fa43b7013ff7b835b6a82c67 100644 (file)
--- a/proc/stat.c
+++ b/proc/stat.c
@@ -990,7 +990,7 @@ PROCPS_EXPORT struct stat_result *procps_stat_get (
         struct stat_info *info,
         enum stat_item item)
 {
-    static time_t sav_secs;
+    static __thread time_t sav_secs;
     time_t cur_secs;
 
     errno = EINVAL;

diff --git a/proc/sysinfo.c b/proc/sysinfo.c
index 79ba6e5c20b56c712ab2d6b7600d78638395df1a..9dd0be03ce7bf77dd0b71378d437970f0231b07a 100644 (file)
--- a/proc/sysinfo.c
+++ b/proc/sysinfo.c
@@ -49,14 +49,14 @@ static int loadavg_fd = -1;
 // and would need 1258 if the obsolete fields were there.
 // As of 3.13 /proc/vmstat needs 2623,
 // and /proc/stat needs 3076.
-static char buf[8192];
+static __thread char buf[8192];
 
 /* This macro opens filename only if necessary and seeks to 0 so
  * that successive calls to the functions are more efficient.
  * It also reads the current contents of the file into the global buf.
  */
 #define FILE_TO_BUF(filename, fd) do{                          \
-    static int local_n;                                                \
+    static __thread int local_n;                                               \
     if (fd == -1 && (fd = open(filename, O_RDONLY)) == -1) {   \
        fputs(BAD_OPEN_MESSAGE, stderr);                        \
        fflush(NULL);                                           \
@@ -158,7 +158,7 @@ PROCPS_EXPORT unsigned int procps_pid_length(void)
 {
     FILE *fp;
     char pidbuf[24];
-    static int pid_length=0;
+    static __thread int pid_length=0;
 
     if (pid_length)
         return pid_length;

diff --git a/proc/vmstat.c b/proc/vmstat.c
index f9334175d87439298683ef15c2e6f566061ce52f..9281cefab23c04354a89a10f777bf319690c12d4 100644 (file)
--- a/proc/vmstat.c
+++ b/proc/vmstat.c
@@ -1193,7 +1193,7 @@ static int vmstat_read_failed (
     head = buf;
 
     for (;;) {
-        static ENTRY e;      // just to keep coverity off our backs (e.data)
+        static __thread ENTRY e;  // keep coverity off our backs (e.data)
         ENTRY *ep;
 
         if (!(tail = strchr(head, ' ')))
@@ -1376,7 +1376,7 @@ PROCPS_EXPORT struct vmstat_result *procps_vmstat_get (
         struct vmstat_info *info,
         enum vmstat_item item)
 {
-    static time_t sav_secs;
+    static __thread time_t sav_secs;
     time_t cur_secs;
 
     errno = EINVAL;

diff --git a/proc/wchan.c b/proc/wchan.c
index 90ba270fd012ad5a3a68063c3a6f62a85a7398fb..b67ab414a8977a604f93ec75baeaa7cc03989c7e 100644 (file)
--- a/proc/wchan.c
+++ b/proc/wchan.c
@@ -27,7 +27,7 @@
 
 
 const char *lookup_wchan (int pid) {
-   static char buf[64];
+   static __thread char buf[64];
    const char *ret = buf;
    ssize_t num;
    int fd;