On a desktop or server the difference between the realtime and monotonic
clocks doesn't matter much, but we've seen cases on Android where
something's run under timeout(1) but killed before it's had a chance to
do anything because the device went into sleep shortly afterwards and by
the time it came back, the timeout had elapsed (as far as the realtime
clock is concerned) but the process had only had a tiny fraction of the
timeout where the system wasn't suspended.
This patch also adds an xparsetimespec() function; at this point the
only remaining users of xparsetime() are in lib, so maybe that should
become static, at least until we have another need for it?
A bigger question is whether timeout(1) needs to offer the user a choice
between the two different clocks? Although monotonic is usally the right
default choice on Android, and I don't have a specific counter-example
to hand, I can imagine that someone might actually mean "wall clock time"
rather than "cpu clock time" when setting a timeout...
Luckily, timer_create() lets us trivially choose between both clocks
if so.
---
lib/lib.h | 1 +
lib/xwrap.c | 4 ++++
scripts/make.sh | 2 +-
toys/other/reboot.c | 6 +++---
toys/other/timeout.c | 29 ++++++++++++-----------------
toys/posix/sleep.c | 6 +++---
6 files changed, 24 insertions(+), 24 deletions(-)
From 1bb13b41c0878644e77745767b54a06f92e03b29 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <e...@google.com>
Date: Thu, 23 Sep 2021 11:45:42 -0700
Subject: [PATCH] timeout: use the monotonic clock.
On a desktop or server the difference between the realtime and monotonic
clocks doesn't matter much, but we've seen cases on Android where
something's run under timeout(1) but killed before it's had a chance to
do anything because the device went into sleep shortly afterwards and by
the time it came back, the timeout had elapsed (as far as the realtime
clock is concerned) but the process had only had a tiny fraction of the
timeout where the system wasn't suspended.
This patch also adds an xparsetimespec() function; at this point the
only remaining users of xparsetime() are in lib, so maybe that should
become static, at least until we have another need for it?
A bigger question is whether timeout(1) needs to offer the user a choice
between the two different clocks? Although monotonic is usally the right
default choice on Android, and I don't have a specific counter-example
to hand, I can imagine that someone might actually mean "wall clock time"
rather than "cpu clock time" when setting a timeout...
Luckily, timer_create() lets us trivially choose between both clocks
if so.
---
lib/lib.h | 1 +
lib/xwrap.c | 4 ++++
scripts/make.sh | 2 +-
toys/other/reboot.c | 6 +++---
toys/other/timeout.c | 29 ++++++++++++-----------------
toys/posix/sleep.c | 6 +++---
6 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/lib/lib.h b/lib/lib.h
index 920f0b3b..2b940600 100644
--- a/lib/lib.h
+++ b/lib/lib.h
@@ -187,6 +187,7 @@ char *xreadlinkat(int dir, char *name);
char *xreadlink(char *name);
double xstrtod(char *s);
long xparsetime(char *arg, long units, long *fraction);
+void xparsetimespec(char *arg, struct timespec *ts);
long long xparsemillitime(char *arg);
void xpidfile(char *name);
void xregcomp(regex_t *preg, char *rexec, int cflags);
diff --git a/lib/xwrap.c b/lib/xwrap.c
index d2680cda..449e7b14 100644
--- a/lib/xwrap.c
+++ b/lib/xwrap.c
@@ -922,6 +922,10 @@ long long xparsemillitime(char *arg)
return (l*1000LL)+ll;
}
+void xparsetimespec(char *arg, struct timespec *ts)
+{
+ ts->tv_sec = xparsetime(arg, 9, &ts->tv_nsec);
+}
// Compile a regular expression into a regex_t
diff --git a/scripts/make.sh b/scripts/make.sh
index f836e044..3caa855d 100755
--- a/scripts/make.sh
+++ b/scripts/make.sh
@@ -108,7 +108,7 @@ then
# and skip nonexistent libraries for it.
> generated/optlibs.dat
- for i in util crypt m resolv selinux smack attr crypto z log iconv
+ for i in util crypt m resolv rt selinux smack attr crypto z log iconv
do
echo "int main(int argc, char *argv[]) {return 0;}" | \
${CROSS_COMPILE}${CC} $CFLAGS $LDFLAGS -xc - -o generated/libprobe $LDASNEEDED -l$i > /dev/null 2>/dev/null &&
diff --git a/toys/other/reboot.c b/toys/other/reboot.c
index ff8ef966..4770d1f1 100644
--- a/toys/other/reboot.c
+++ b/toys/other/reboot.c
@@ -29,13 +29,13 @@ GLOBALS(
void reboot_main(void)
{
- struct timespec tv;
+ struct timespec ts;
int types[] = {RB_AUTOBOOT, RB_HALT_SYSTEM, RB_POWER_OFF},
sigs[] = {SIGTERM, SIGUSR1, SIGUSR2}, idx;
if (TT.d) {
- tv.tv_sec = xparsetime(TT.d, 9, &tv.tv_nsec);
- nanosleep(&tv, NULL);
+ xparsetimespec(TT.d, &ts);
+ nanosleep(&ts, NULL);
}
if (!FLAG(n)) sync();
diff --git a/toys/other/timeout.c b/toys/other/timeout.c
index 1e125300..0edea61b 100644
--- a/toys/other/timeout.c
+++ b/toys/other/timeout.c
@@ -33,8 +33,9 @@ GLOBALS(
int nextsig;
pid_t pid;
- struct timeval ktv;
- struct itimerval itv;
+ struct timespec kts;
+ struct itimerspec its;
+ timer_t timer;
)
static void handler(int i)
@@ -48,27 +49,21 @@ static void handler(int i)
TT.k = 0;
TT.nextsig = SIGKILL;
xsignal(SIGALRM, handler);
- TT.itv.it_value = TT.ktv;
- setitimer(ITIMER_REAL, &TT.itv, (void *)toybuf);
+ TT.its.it_value = TT.kts;
+ if (timer_settime(TT.timer, 0, &TT.its, 0)) perror_exit("timer_settime");
}
}
-// timeval inexplicably makes up a new type for microseconds, despite timespec's
-// nanoseconds field (needing to store 1000* the range) using "long". Bravo.
-static void xparsetimeval(char *s, struct timeval *tv)
-{
- long ll;
-
- tv->tv_sec = xparsetime(s, 6, &ll);
- tv->tv_usec = ll;
-}
-
void timeout_main(void)
{
+ struct sigevent se = { .sigev_notify=SIGEV_SIGNAL, .sigev_signo=SIGALRM };
+
// Use same ARGFAIL value for any remaining parsing errors
toys.exitval = 125;
- xparsetimeval(*toys.optargs, &TT.itv.it_value);
- if (TT.k) xparsetimeval(TT.k, &TT.ktv);
+ xparsetimespec(*toys.optargs, &TT.its.it_value);
+ if (TT.k) xparsetimespec(TT.k, &TT.kts);
+
+ if (timer_create(CLOCK_MONOTONIC, &se, &TT.timer)) perror_exit("timer");
TT.nextsig = SIGTERM;
if (TT.s && -1 == (TT.nextsig = sig_to_num(TT.s)))
@@ -82,7 +77,7 @@ void timeout_main(void)
int status;
xsignal(SIGALRM, handler);
- setitimer(ITIMER_REAL, &TT.itv, (void *)toybuf);
+ if (timer_settime(TT.timer, 0, &TT.its, 0)) perror_exit("timer_settime");
status = xwaitpid(TT.pid);
if (FLAG(preserve_status) || !toys.exitval) toys.exitval = status;
diff --git a/toys/posix/sleep.c b/toys/posix/sleep.c
index 846df80c..73f03fb4 100644
--- a/toys/posix/sleep.c
+++ b/toys/posix/sleep.c
@@ -23,8 +23,8 @@ config SLEEP
void sleep_main(void)
{
- struct timespec tv;
+ struct timespec ts;
- tv.tv_sec = xparsetime(*toys.optargs, 9, &tv.tv_nsec);
- toys.exitval = !!nanosleep(&tv, NULL);
+ xparsetimespec(*toys.optargs, &ts);
+ toys.exitval = !!nanosleep(&ts, NULL);
}
--
2.33.0.685.g46640cef36-goog
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
