From eafd8e31124697be859a3d305e72b03b9be8bc95 Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Wed, 10 Nov 2021 00:00:00 -0500 Subject: [PATCH] library: refine support for multiple concurrent access Our new library's now well protected against potential problems which arise when a multi-threaded application opens more than one context within the same API at the same time. However, with a single-threaded application designed along those same lines, some problems remain. So, to avoid potential corruption of some data (which was classified as local 'static __thread') from those single-threaded designs, we'll move several variables to the info structure itself and remove the '__thread' qualifier. Now they're protected against both designs. [ we'll not be protected against some multi-threaded ] [ application that shares a single context yet calls ] [ that interface from separate threads. this is just ] [ bad application design & no different than sharing ] [ other modifiable global data between such threads! ] Signed-off-by: Jim Warner --- proc/meminfo.c | 6 +++--- proc/pids.c | 14 +++++++------- proc/slabinfo.c | 6 +++--- proc/stat.c | 6 +++--- proc/vmstat.c | 6 +++--- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/proc/meminfo.c b/proc/meminfo.c index 705e5122..79e7fe19 100644 --- a/proc/meminfo.c +++ b/proc/meminfo.c @@ -135,6 +135,7 @@ struct meminfo_info { struct stacks_extent *extents; struct hsearch_data hashtab; struct meminfo_result get_this; + time_t sav_secs; }; @@ -882,7 +883,6 @@ PROCPS_EXPORT struct meminfo_result *procps_meminfo_get ( struct meminfo_info *info, enum meminfo_item item) { - static __thread time_t sav_secs; time_t cur_secs; errno = EINVAL; @@ -895,10 +895,10 @@ PROCPS_EXPORT struct meminfo_result *procps_meminfo_get ( /* we will NOT read the meminfo file with every call - rather, we'll offer a granularity of 1 second between reads ... */ cur_secs = time(NULL); - if (1 <= cur_secs - sav_secs) { + if (1 <= cur_secs - info->sav_secs) { if (meminfo_read_failed(info)) return NULL; - sav_secs = cur_secs; + info->sav_secs = cur_secs; } info->get_this.item = item; diff --git a/proc/pids.c b/proc/pids.c index 778f2a90..f932ae51 100644 --- a/proc/pids.c +++ b/proc/pids.c @@ -96,6 +96,8 @@ struct pids_info { struct stacks_extent *get_ext; // for active 'get' (also within 'extents') enum pids_fetch_type get_type; // last known type of 'get' request int seterr; // an ENOMEM encountered during assign + proc_t get_proc; // the proc_t used by procps_pids_get + proc_t fetch_proc; // the proc_t used by pids_stacks_fetch }; @@ -1123,7 +1125,6 @@ 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 __thread proc_t task; // static for initial 0's + later free(s) struct stacks_extent *ext; // initialize stuff ----------------------------------- @@ -1140,7 +1141,7 @@ static int pids_stacks_fetch ( // iterate stuff -------------------------------------- n_inuse = 0; - while (info->read_something(info->fetch_PT, &task)) { + while (info->read_something(info->fetch_PT, &info->fetch_proc)) { if (!(n_inuse < n_alloc)) { n_alloc += STACKS_GROW; if (!(info->fetch.anchor = realloc(info->fetch.anchor, sizeof(void *) * n_alloc)) @@ -1148,9 +1149,9 @@ static int pids_stacks_fetch ( return -1; // here, errno was set to ENOMEM memcpy(info->fetch.anchor + n_inuse, ext->stacks, sizeof(void *) * STACKS_GROW); } - if (!pids_proc_tally(info, &info->fetch.counts, &task)) + if (!pids_proc_tally(info, &info->fetch.counts, &info->fetch_proc)) return -1; // here, errno was set to ENOMEM - if (!pids_assign_results(info, info->fetch.anchor[n_inuse++], &task)) + if (!pids_assign_results(info, info->fetch.anchor[n_inuse++], &info->fetch_proc)) return -1; // here, errno was set to ENOMEM } /* while the possibility is extremely remote, the readproc.c (read_something) | @@ -1404,7 +1405,6 @@ PROCPS_EXPORT struct pids_stack *procps_pids_get ( struct pids_info *info, enum pids_fetch_type which) { - static __thread proc_t task; // static for initial 0's + later free(s) errno = EINVAL; if (info == NULL) @@ -1432,9 +1432,9 @@ fresh_start: } errno = 0; - if (NULL == info->read_something(info->get_PT, &task)) + if (NULL == info->read_something(info->get_PT, &info->get_proc)) return NULL; - if (!pids_assign_results(info, info->get_ext->stacks[0], &task)) + if (!pids_assign_results(info, info->get_ext->stacks[0], &info->get_proc)) return NULL; return info->get_ext->stacks[0]; } // end: procps_pids_get diff --git a/proc/slabinfo.c b/proc/slabinfo.c index 59e4000e..bc048eef 100644 --- a/proc/slabinfo.c +++ b/proc/slabinfo.c @@ -133,6 +133,7 @@ struct slabinfo_info { struct fetch_support fetch; // support for procps_slabinfo_reap struct slabs_node nul_node; // used by slabinfo_get/select struct slabinfo_result get_this; // used by slabinfo_get + time_t sav_secs; // used by slabinfo_get }; @@ -839,7 +840,6 @@ PROCPS_EXPORT struct slabinfo_result *procps_slabinfo_get ( struct slabinfo_info *info, enum slabinfo_item item) { - static __thread time_t sav_secs; time_t cur_secs; errno = EINVAL; @@ -852,10 +852,10 @@ PROCPS_EXPORT struct slabinfo_result *procps_slabinfo_get ( /* we will NOT read the slabinfo file with every call - rather, we'll offer a granularity of 1 second between reads ... */ cur_secs = time(NULL); - if (1 <= cur_secs - sav_secs) { + if (1 <= cur_secs - info->sav_secs) { if (slabinfo_read_failed(info)) return NULL; - sav_secs = cur_secs; + info->sav_secs = cur_secs; } info->get_this.item = item; diff --git a/proc/stat.c b/proc/stat.c index e2ec4a3a..8ce6f5c7 100644 --- a/proc/stat.c +++ b/proc/stat.c @@ -145,6 +145,7 @@ struct stat_info { struct stat_result get_this; // for return to caller after a get struct item_support reap_items; // items used for reap (shared among 3) struct item_support select_items; // items unique to select + time_t sav_secs; // used by procps_stat_get to limit i/o }; @@ -990,7 +991,6 @@ PROCPS_EXPORT struct stat_result *procps_stat_get ( struct stat_info *info, enum stat_item item) { - static __thread time_t sav_secs; time_t cur_secs; errno = EINVAL; @@ -1003,10 +1003,10 @@ PROCPS_EXPORT struct stat_result *procps_stat_get ( /* we will NOT read the source file with every call - rather, we'll offer a granularity of 1 second between reads ... */ cur_secs = time(NULL); - if (1 <= cur_secs - sav_secs) { + if (1 <= cur_secs - info->sav_secs) { if (stat_read_failed(info)) return NULL; - sav_secs = cur_secs; + info->sav_secs = cur_secs; } info->get_this.item = item; diff --git a/proc/vmstat.c b/proc/vmstat.c index 9281cefa..7256cf09 100644 --- a/proc/vmstat.c +++ b/proc/vmstat.c @@ -236,6 +236,7 @@ struct vmstat_info { struct stacks_extent *extents; struct hsearch_data hashtab; struct vmstat_result get_this; + time_t sav_secs; }; @@ -1376,7 +1377,6 @@ PROCPS_EXPORT struct vmstat_result *procps_vmstat_get ( struct vmstat_info *info, enum vmstat_item item) { - static __thread time_t sav_secs; time_t cur_secs; errno = EINVAL; @@ -1389,10 +1389,10 @@ PROCPS_EXPORT struct vmstat_result *procps_vmstat_get ( /* we will NOT read the vmstat file with every call - rather, we'll offer a granularity of 1 second between reads ... */ cur_secs = time(NULL); - if (1 <= cur_secs - sav_secs) { + if (1 <= cur_secs - info->sav_secs) { if (vmstat_read_failed(info)) return NULL; - sav_secs = cur_secs; + info->sav_secs = cur_secs; } info->get_this.item = item;