From 082355c0f2afdc9d51d5aeae47e4a90c79de0390 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 5 Sep 2018 17:15:00 -0400 Subject: [PATCH] Improve process checking on 32-bit systems. --- 0001-Handle-failure-from-sysconf.patch | 42 ++++++++ ...-the-sense-of-psll_linux_clock_ticks.patch | 98 +++++++++++++++++++ ...er-comparison-for-process-start-time.patch | 64 ++++++++++++ R-ps.spec | 15 ++- 4 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 0001-Handle-failure-from-sysconf.patch create mode 100644 0002-Invert-the-sense-of-psll_linux_clock_ticks.patch create mode 100644 0003-Use-safer-comparison-for-process-start-time.patch diff --git a/0001-Handle-failure-from-sysconf.patch b/0001-Handle-failure-from-sysconf.patch new file mode 100644 index 0000000..59b8698 --- /dev/null +++ b/0001-Handle-failure-from-sysconf.patch @@ -0,0 +1,42 @@ +From 00349771f69aa23f4ece4d0ec42566755460fce2 Mon Sep 17 00:00:00 2001 +From: Elliott Sales de Andrade +Date: Wed, 5 Sep 2018 04:50:12 -0400 +Subject: [PATCH 1/3] Handle failure from sysconf. + +Signed-off-by: Elliott Sales de Andrade +--- + src/api-linux.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/src/api-linux.c b/src/api-linux.c +index f2995df..02f6268 100644 +--- a/src/api-linux.c ++++ b/src/api-linux.c +@@ -261,8 +261,12 @@ int psll_linux_get_boot_time() { + return 0; + } + +-int psll_linux_get_clock_ticks() { ++int psll_linux_get_clock_ticks(void) { + psll_linux_clock_ticks = sysconf(_SC_CLK_TCK); ++ if (psll_linux_clock_ticks == -1) { ++ ps__set_error_from_errno(); ++ return -1; ++ } + return 0; + } + +@@ -278,7 +282,9 @@ int psll_linux_ctime(long pid, double *ctime) { + + if (!psll_linux_clock_ticks) { + ret = psll_linux_get_clock_ticks(); +- if (ret) return ret; ++ if (ret) { ++ ps__throw_error(); ++ } + } + + *ctime = psll_linux_boot_time + stat.starttime / psll_linux_clock_ticks; +-- +2.17.1 + diff --git a/0002-Invert-the-sense-of-psll_linux_clock_ticks.patch b/0002-Invert-the-sense-of-psll_linux_clock_ticks.patch new file mode 100644 index 0000000..9ae0686 --- /dev/null +++ b/0002-Invert-the-sense-of-psll_linux_clock_ticks.patch @@ -0,0 +1,98 @@ +From 161c21384b91799c2b26f1aa776e081eed002add Mon Sep 17 00:00:00 2001 +From: Elliott Sales de Andrade +Date: Wed, 5 Sep 2018 13:51:43 -0400 +Subject: [PATCH 2/3] Invert the sense of psll_linux_clock_ticks. + +It is always used to divide some other number, so instead save its +reciprocal. Since division is generally slower than multiplication, +doing one reciprocal and multiplication everywhere else should be +slightly faster. + +Signed-off-by: Elliott Sales de Andrade +--- + src/api-linux.c | 25 +++++++++++++------------ + 1 file changed, 13 insertions(+), 12 deletions(-) + +diff --git a/src/api-linux.c b/src/api-linux.c +index 02f6268..d2b6193 100644 +--- a/src/api-linux.c ++++ b/src/api-linux.c +@@ -15,7 +15,7 @@ + #include "common.h" + + double psll_linux_boot_time = 0; +-double psll_linux_clock_ticks = 0; ++double psll_linux_clock_period = 0; + + typedef struct { + char state; +@@ -29,7 +29,7 @@ typedef struct { + #define PS__TV2DOUBLE(t) ((t).tv_sec + (t).tv_usec / 1000000.0) + + #define PS__CHECK_STAT(stat, handle) \ +- if (psll_linux_boot_time + (stat).starttime / psll_linux_clock_ticks != \ ++ if (psll_linux_boot_time + (stat).starttime * psll_linux_clock_period != \ + handle->create_time) { \ + ps__no_such_process(handle->pid, 0); \ + ps__throw_error(); \ +@@ -226,7 +226,7 @@ void ps__check_for_zombie(ps_handle_t *handle, int err) { + ps__throw_error(); + } + +- if (psll_linux_boot_time + stat.starttime / psll_linux_clock_ticks != ++ if (psll_linux_boot_time + stat.starttime * psll_linux_clock_period != + handle->create_time) { + ps__no_such_process(handle->pid, 0); + err = 1; +@@ -261,12 +261,13 @@ int psll_linux_get_boot_time() { + return 0; + } + +-int psll_linux_get_clock_ticks(void) { +- psll_linux_clock_ticks = sysconf(_SC_CLK_TCK); ++int psll_linux_get_clock_period(void) { ++ double psll_linux_clock_ticks = sysconf(_SC_CLK_TCK); + if (psll_linux_clock_ticks == -1) { + ps__set_error_from_errno(); + return -1; + } ++ psll_linux_clock_period = 1.0 / psll_linux_clock_ticks; + return 0; + } + +@@ -280,14 +281,14 @@ int psll_linux_ctime(long pid, double *ctime) { + if (ret) return ret; + } + +- if (!psll_linux_clock_ticks) { +- ret = psll_linux_get_clock_ticks(); ++ if (!psll_linux_clock_period) { ++ ret = psll_linux_get_clock_period(); + if (ret) { + ps__throw_error(); + } + } + +- *ctime = psll_linux_boot_time + stat.starttime / psll_linux_clock_ticks; ++ *ctime = psll_linux_boot_time + stat.starttime * psll_linux_clock_period; + + return 0; + } +@@ -716,10 +717,10 @@ SEXP psll_cpu_times(SEXP p) { + PS__CHECK_STAT(stat, handle); + + PROTECT(result = allocVector(REALSXP, 4)); +- REAL(result)[0] = stat.utime / psll_linux_clock_ticks; +- REAL(result)[1] = stat.stime / psll_linux_clock_ticks; +- REAL(result)[2] = stat.cutime / psll_linux_clock_ticks; +- REAL(result)[3] = stat.cstime / psll_linux_clock_ticks; ++ REAL(result)[0] = stat.utime * psll_linux_clock_period; ++ REAL(result)[1] = stat.stime * psll_linux_clock_period; ++ REAL(result)[2] = stat.cutime * psll_linux_clock_period; ++ REAL(result)[3] = stat.cstime * psll_linux_clock_period; + PROTECT(names = ps__build_string("user", "system", "childen_user", + "children_system", NULL)); + setAttrib(result, R_NamesSymbol, names); +-- +2.17.1 + diff --git a/0003-Use-safer-comparison-for-process-start-time.patch b/0003-Use-safer-comparison-for-process-start-time.patch new file mode 100644 index 0000000..0b8f983 --- /dev/null +++ b/0003-Use-safer-comparison-for-process-start-time.patch @@ -0,0 +1,64 @@ +From 37da965d8ae37bb9f00dcced97d44bd446896849 Mon Sep 17 00:00:00 2001 +From: Elliott Sales de Andrade +Date: Wed, 5 Sep 2018 14:27:46 -0400 +Subject: [PATCH 3/3] Use safer comparison for process start time. + +Comparing two doubles for equality can be troublesome, especially on +32-bit systems (possibly due to 80-bit registers). Checking for some +epsilon is safer. + +Signed-off-by: Elliott Sales de Andrade +--- + src/api-linux.c | 22 ++++++++++++++-------- + 1 file changed, 14 insertions(+), 8 deletions(-) + +diff --git a/src/api-linux.c b/src/api-linux.c +index d2b6193..8162f1d 100644 +--- a/src/api-linux.c ++++ b/src/api-linux.c +@@ -28,12 +28,16 @@ typedef struct { + + #define PS__TV2DOUBLE(t) ((t).tv_sec + (t).tv_usec / 1000000.0) + +-#define PS__CHECK_STAT(stat, handle) \ +- if (psll_linux_boot_time + (stat).starttime * psll_linux_clock_period != \ +- handle->create_time) { \ +- ps__no_such_process(handle->pid, 0); \ +- ps__throw_error(); \ +- } ++#define PS__CHECK_STAT(stat, handle) \ ++ do { \ ++ double starttime = psll_linux_boot_time + \ ++ (stat).starttime * psll_linux_clock_period; \ ++ double diff = starttime - (handle)->create_time; \ ++ if (fabs(diff) > psll_linux_clock_period) { \ ++ ps__no_such_process((handle)->pid, 0); \ ++ ps__throw_error(); \ ++ } \ ++ } while (0) + + #define PS__CHECK_HANDLE(handle) \ + do { \ +@@ -218,6 +222,7 @@ int psll__parse_stat_file(long pid, psl_stat_t *stat, char **name) { + + void ps__check_for_zombie(ps_handle_t *handle, int err) { + psl_stat_t stat; ++ double diff; + + if (!handle) error("Process pointer cleaned up already"); + +@@ -226,8 +231,9 @@ void ps__check_for_zombie(ps_handle_t *handle, int err) { + ps__throw_error(); + } + +- if (psll_linux_boot_time + stat.starttime * psll_linux_clock_period != +- handle->create_time) { ++ diff = (psll_linux_boot_time + stat.starttime * psll_linux_clock_period) - ++ handle->create_time; ++ if (fabs(diff) > psll_linux_clock_period) { + ps__no_such_process(handle->pid, 0); + err = 1; + } else if (stat.state == 'Z') { +-- +2.17.1 + diff --git a/R-ps.spec b/R-ps.spec index 67d3870..27b85ba 100644 --- a/R-ps.spec +++ b/R-ps.spec @@ -6,12 +6,16 @@ Name: R-%{packname} Version: 1.1.0 -Release: 2%{?dist} +Release: 3%{?dist} Summary: List, Query, Manipulate System Processes License: BSD URL: https://cran.r-project.org/web/packages/%{packname}/index.html Source0: https://cran.r-project.org/src/contrib/%{packname}_%{version}.tar.gz +# https://github.com/r-lib/ps/pull/42 +Patch0001: 0001-Handle-failure-from-sysconf.patch +Patch0002: 0002-Invert-the-sense-of-psll_linux_clock_ticks.patch +Patch0003: 0003-Use-safer-comparison-for-process-start-time.patch # Here's the R view of the dependencies world: # Depends: @@ -42,6 +46,12 @@ List, query and manipulate all system processes, on 'Windows', 'Linux' and %prep %setup -q -c -n %{packname} +pushd %{packname} +%patch0001 -p1 +%patch0002 -p1 +%patch0003 -p1 +popd + # Don't need coverage; it's not packaged either. sed -i 's/covr, //g' %{packname}/DESCRIPTION @@ -82,6 +92,9 @@ _R_CHECK_FORCE_SUGGESTS_=0 %{_bindir}/R CMD check %{packname} --no-tests %changelog +* Wed Sep 05 2018 Elliott Sales de Andrade - 1.1.0-3 +- Improve process checking on 32-bit systems + * Mon Aug 20 2018 Elliott Sales de Andrade - 1.1.0-2 - Add option to disable tests due to dependency loop