Improve process checking on 32-bit systems.

This commit is contained in:
Elliott Sales de Andrade 2018-09-05 17:15:00 -04:00
parent fefae43bc5
commit 082355c0f2
4 changed files with 218 additions and 1 deletions

View File

@ -0,0 +1,42 @@
From 00349771f69aa23f4ece4d0ec42566755460fce2 Mon Sep 17 00:00:00 2001
From: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Date: Wed, 5 Sep 2018 04:50:12 -0400
Subject: [PATCH 1/3] Handle failure from sysconf.
Signed-off-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
---
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

View File

@ -0,0 +1,98 @@
From 161c21384b91799c2b26f1aa776e081eed002add Mon Sep 17 00:00:00 2001
From: Elliott Sales de Andrade <quantum.analyst@gmail.com>
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 <quantum.analyst@gmail.com>
---
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

View File

@ -0,0 +1,64 @@
From 37da965d8ae37bb9f00dcced97d44bd446896849 Mon Sep 17 00:00:00 2001
From: Elliott Sales de Andrade <quantum.analyst@gmail.com>
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 <quantum.analyst@gmail.com>
---
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

View File

@ -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 <quantum.analyst@gmail.com> - 1.1.0-3
- Improve process checking on 32-bit systems
* Mon Aug 20 2018 Elliott Sales de Andrade <quantum.analyst@gmail.com> - 1.1.0-2
- Add option to disable tests due to dependency loop